Make chord representation consistent

[majg0]

This is tricky! :P I really appreciate your hard work on this crate!

In

rust-music-theory/src/chord/chord.rs

Line 92 in 86591a1

intervals: vec![],

, the default chord is erroneously missing its intervals.

The easiest way to fix this is to use ::new and call it a day, but imo that keeps the design open for a similar problem happening elsewhere.

Another viable approach:
What if we make Chord a trait instead, and make several concrete representations for it?
For example a vector of intervals could be considered a chord, albeit without a root pitch class. Chord quality and whether it's a triad/seventh /something exotic could be inferred from the intervals.

We could still construct chords from quality, string, etc but my point is we should ideally only represent it with one unique and flexible data point, because right now it's possible for the Chord struct to contain invalid data, where the fields mismatch each other and go out of sync.

If we consider performance, I'd personally vouch for being able to compute each thing from each other thing, e.g. string representation from intervals, quality from string, etc.

What do you think?

[ozankasikci]

@martingronlund thank you.

I agree that a chord should never have an empty list of intervals. However i'm not sure how would making the Chord a trait prevent that? The implementation of a Chord trait could still consist an empty vector of intervals, unless it's explicitly checked.

What we can and will do instead is, returning Result<Self> from Chord::new. This is the idiomatic way to handle invalid data in Rust.

Does that make sense to you?

[majg0]

Not really. The point I was trying to make is that there is redundant data, which means that the struct can become invalid at any time if we're unlucky or not careful enough. I guess you want to cache some fields for performance, but if those go unaccessed it might just be a hog of unnecessary resources. Imo better to compute on demand and be in control of caching.

[ozankasikci]

Hi Martin, can you please explain what you mean with some concrete code examples? because i'm not sure if i'm following what you mean here.

[majg0]

I'll try :)

Let's say we want to represent a C major chord.

• We could say that it's the note C with a "Major" quality, which could e.g. be an enum entry or a collection of intervals or a bit field or something.
• We could say it's the note collection {C, E, G}. We could store this e.g. using a set or e.g. a vec.

My point is there are many ways to fully represent just "C major" - that very string being yet another example of how to fully encode all information we need (assuming western music with equal temperament etc etc, which is fair).

The main point I want to make, and the core concept I think is important here, is that we should avoid mixing several representations of the same conceptual data in the core library structs.
E.g. we should avoid storing all of { root note (C) + intervals (M3,P5) + notes (C,E,G) } in a chord struct, because it contains redundant data. There are several reasons for me to think its inadvisable to mix representations;

• I think there could be countless things we might want to add to a chord struct and adding everything would make it slow to construct and move around in memory. E.g. what chord does it lead well into, with weights for how strongly? Is it consonant or dissonant sounding? What emotions are commonly attributed to it (e.g. happy)? Etc
• Any unique representation (e.g. C + quality) can be used to compute the other fields. These other fields can therefore be considered as just being cached data. That cached data might never be used, or it might even worse go stale and lead to inconsistent state, as has already happened once. I think it's inevitable that these "caches which are not clearly caches" WILL go stale, because I've seen it a lot and I have come to think it has do with our human inability to think of every consequence of every change in state, so nobody is to blame.
• Small orthogonal pieces of data to work with usually seem to scale better in terms of composability and expressivity. In my experience, big projects which contain big structs are almost always messy to work with, and I think it again has to do with our inherent disability to understand the consequences of state changes. State changes are easier to understand when the state which is changed is well contained and its boundary is clearly defined.

My main proposal is simple:
All redundant data could be computed on demand.

This would be reducing construction overhead and memory consumption, making state more easily understandable and easy to reason about, while giving API users more control.

Now... In my previous comments I couldn't settle on how to best represent a chord fundamentally and I thought that, in some sense, the string "C major" is just as good as {C + major quality}, so I proposed using a trait to gather all those representations of a chord into one. However, I think I was wrong: "C major" is a worse representation than {C + quality}.

I'd like to propose

1. struct Chord { root: Note, quality: u16 }
2. Move the old fields which I removed, into functions. Users can cache on demand instead.

Then using the sixteen bits as flags for whether the interval of the corresponding distance is "on". E.g. in C major, there is a M3 and P5, which are 4 and 7 semitones away from C respectively, which translates to e.g. 1 << 3 == 8 and 1 << 6 == 64 so 8 + 64 == 72. Alternatively, not assuming the root is always present like I just did, reserving bit 1 for whether it is: 1 + 1 << 4 + 1 << 7 == 145. I think this second approach is nicer because of additional flexibility together with that the numbers for bitshifts line up with the semitone count.

Phew. 👍 haha

[ozankasikci]

Thanks for elaborating, Martin! :)

Now i know your reasoning and what you meant. This sounds like a more flexible way of representing chords. But of course would require some work, and design decisions. Also would dramatically change the API that is exposed.

I'm not against the idea of having this dynamic way of representing a chord. Might implement this sometime in future. Atm i don't have the time for it and the road map is already defined a long time ago.

I appreciate your time and effort for proposing this!

[donRumata03]

This idea looks pretty much the same to the core concept of normalized data bases: if there's a redundancy in you data representation, it will finally go out of synchronization.