Builder pattern for scales and chords

[XBagon]

I would propose to use the builder pattern for those, because for most fields there are good defaults, we could add new ways to build them and especially for the case of calculating the inversion from a bass_note. I would like to expose that feature to the library side and not only have it in the CLI.
I suggest the typed-builder crate for this, as there is no need to write the boilerplate by hand and it also brings the compile time check.

[henryksloan]

The inversion by note and number are each handled by the from_regex function of Chord, so they are technically exposed to the library. That said, I agree it would be nice to expose that functionality as a separate function. I'm not super familiar with the builder pattern. I get the general idea, but could you flesh out the specific benefits for chords and scales?

That crate looks quite good if we decide to go this path.

[XBagon]

It would allow to construct scales/chords more quickly, cause you wouldn't need to specify the octave for example, it would just default to 4. Yes, you could do that with ..Default::default(), but I'd still say it's nicer and there are other benefits. You can have different ways of setting fields, like in the mentioned case of inversions. Also you could make it possible to build up chords like this:

let chord = ChordBuilder::new()
    .root(Pitch::new(C, 0))
    .build_interval(Major,Third)
    .build_interval(Perfect,Fifth)
    .build_interval(Major,Seventh)
    .build();

[henryksloan]

Wow, that's actually a super cool idea. To be clear, could that have required fields, such as root? Also, for the sake of user-friendliness, I think it is very important to support chord names like we already do, as in Chord::new(C, Major, Seventh.... Here's an idea: We could use the method you recommend, but make it a trait called NotesBuilder. Then, ChordBuilder and ScaleBuilder could implement and extend this trait with functions related to creating and using chords/scales. These two builders would respectively produce Chords and Scales, which each already implement Notes. The pattern you're suggesting is new to me, so maybe I'm way off, but I think my proposal could solve some issues. The builder could support altering notes, so we could have something like:

let chord = ChordBuilder::new()
    .root(Pitch::new(C, 0))
    .quality(Major)
    .number(Seventh)
    .alter(Fourth, 1) // sharp 11th
    .build();
// chord: {C, E, G, B, F#}

In that example, NoteSetBuilder::alter would check if the chord has a Fourth already, and if not, adds one with the given alteration (1 = #). If the chord already had a Fourth, it would set the alteration (or add to it??).

I think ScaleBuilder could also have various methods such as type(pentatonic). Combined with good defaults, that could be a very usable and extensible framework:

let scale = ScaleBuilder::new()
    .root(Pitch::new(A, 0))
    .mode(Minor)
    .type(Pentatonic)
    .alter(Seventh, 1)
    .build()
// scale: {A, C, D, E, G#}

Just throwing ideas at the wall, so please let me know what you think. Good idea!

[XBagon]

Required fields are possible yes!
I am not sure what would try to accomplish with the trait, which functions would you want to share and why? I am in favour of implementing them with very similarly API, just because it makes sense and is easy to remember, but I don't see what the trait would achieve yet. When build is called, it just returns the structs, which are implementing the Notes trait anyway, if that's what you were going for?

Altering notes like that is definitely a good idea! About the pentatonic "type", that's what I was aiming for with #11, I'd say the base functionality has to be implemented first before adding fitting functions to the builder.

I think you got the gist of it and I'm glad you agree on the advantages!

[henryksloan]

Ah, I was actually thinking of #11 when I mentioned different scale types, but I didn't know it was your post! You may be right that the trait implementation is unnecessary, but I'm envisioning that the code for building chords and scales could be almost identical in all but some specifics. If there's an opportunity to share the majority of the code, I think it's a good option, but we'll see if that's the case.

I'm thinking that NotesBuilder could implement root, alter, build_interval (or something like that), and could have a virtual build function. After all, this builder will most likely be implemented as a Vec or HashSet of notes, so the only things specific to chords and scales are particular modifications of the notes in the note set.

You also mentioned that we could easily implement inversions with this builder pattern. I agree, but that calls into question how this change would affect the internals of Chord and Scale. Since this would allow for very complex, novel chords and scales to be made, are you imagining that Chord and Scale would just wrap a list of notes?