Added explicit raw values to ChordType #48
Conversation
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
|
|
||
| /// Suspended Fourth Add Sharp Thirteen: Perfect Fourth, Perfect Fifth, Augmented Thirteenth, e.g. `Csus4(add#13)` | ||
| case sus4_addSharp13 | ||
| case sus4_addSharp13 = "sus4_addSharp13" |
There was a problem hiding this comment.
Identifier Name Violation: Enum element name should only contain alphanumeric characters: 'sus4_addSharp13' (identifier_name)
Redundant String Enum Value Violation: String enum values can be omitted when they are equal to the enumcase name. (redundant_string_enum_value)
|
|
||
| /// Suspended Second Add Sharp Thirteen: Major Second, Perfect Fifth, Augmented Thirteenth, e.g. `Csus2(add#13)` | ||
| case sus2_addSharp13 | ||
| case sus2_addSharp13 = "sus2_addSharp13" |
There was a problem hiding this comment.
Identifier Name Violation: Enum element name should only contain alphanumeric characters: 'sus2_addSharp13' (identifier_name)
Redundant String Enum Value Violation: String enum values can be omitted when they are equal to the enumcase name. (redundant_string_enum_value)
|
|
||
| /// Suspended 4th Add Flat Thirteen: Major Second, Perfect Fifth, Minor Thirteenth, e.g. `Csus4(add♭13)` | ||
| case sus4_addFlat13 | ||
| case sus4_addFlat13 = "sus4_addFlat13" |
There was a problem hiding this comment.
Identifier Name Violation: Enum element name should only contain alphanumeric characters: 'sus4_addFlat13' (identifier_name)
Redundant String Enum Value Violation: String enum values can be omitted when they are equal to the enumcase name. (redundant_string_enum_value)
|
|
||
| /// Suspended 2nd Add Flat Thirteen: Major Second, Perfect Fifth, Minor Thirteenth, e.g. `Csus2(add♭13)` | ||
| case sus2_addFlat13 | ||
| case sus2_addFlat13 = "sus2_addFlat13" |
There was a problem hiding this comment.
Identifier Name Violation: Enum element name should only contain alphanumeric characters: 'sus2_addFlat13' (identifier_name)
Redundant String Enum Value Violation: String enum values can be omitted when they are equal to the enumcase name. (redundant_string_enum_value)
|
|
||
| /// Suspended 4th Add Thirteen: Major Fourth, Perfect Fifth, Major Sixth, e.g. `Csus4(add13)` | ||
| case sus4_add13 | ||
| case sus4_add13 = "sus4_add13" |
There was a problem hiding this comment.
Identifier Name Violation: Enum element name should only contain alphanumeric characters: 'sus4_add13' (identifier_name)
Redundant String Enum Value Violation: String enum values can be omitted when they are equal to the enumcase name. (redundant_string_enum_value)
|
|
||
| /// Major Seventh Suspended Fourth Sharp Ninth: Perfect Fourth, Perfect Fifth, Major Seventh, Augmented Ninth, e.g. `Cmaj7sus4(♯9)` | ||
| case maj7_sus4_sharp9 | ||
| case maj7_sus4_sharp9 = "maj7_sus4_sharp9" |
There was a problem hiding this comment.
Identifier Name Violation: Enum element name should only contain alphanumeric characters: 'maj7_sus4_sharp9' (identifier_name)
Redundant String Enum Value Violation: String enum values can be omitted when they are equal to the enumcase name. (redundant_string_enum_value)
|
|
||
| /// Major Seventh Suspended Fourth Flat Ninth: Perfect Fourth, Perfect Fifth, Major Seventh, Minor Ninth, e.g. `Cmaj7sus4(♭9)` | ||
| case maj7_sus4_flat9 | ||
| case maj7_sus4_flat9 = "maj7_sus4_flat9" |
There was a problem hiding this comment.
Identifier Name Violation: Enum element name should only contain alphanumeric characters: 'maj7_sus4_flat9' (identifier_name)
Redundant String Enum Value Violation: String enum values can be omitted when they are equal to the enumcase name. (redundant_string_enum_value)
|
|
||
| /// Dominant Thirteenth Suspended Second Sharp Eleventh: Major Second, Perfect Fifth, Minor Seventh, Major Ninth, Augmented Eleventh, Major Thirteenth, e.g. `C13sus2(♯11)` | ||
| case dom13_sus2_sharp11 | ||
| case dom13_sus2_sharp11 = "dom13_sus2_sharp11" |
There was a problem hiding this comment.
Identifier Name Violation: Enum element name should only contain alphanumeric characters: 'dom13_sus2_sharp11' (identifier_name)
Redundant String Enum Value Violation: String enum values can be omitted when they are equal to the enumcase name. (redundant_string_enum_value)
|
|
||
| /// Dominant Thirteenth Suspended Fourth Sharp Ninth: Perfect Fourth, Perfect Fifth, Minor Seventh, Augmented Ninth, Perfect Eleventh, Major Thirteenth, e.g. `C13sus4(♯9)` | ||
| case dom13_sus4_sharp9 | ||
| case dom13_sus4_sharp9 = "dom13_sus4_sharp9" |
There was a problem hiding this comment.
Identifier Name Violation: Enum element name should only contain alphanumeric characters: 'dom13_sus4_sharp9' (identifier_name)
Redundant String Enum Value Violation: String enum values can be omitted when they are equal to the enumcase name. (redundant_string_enum_value)
|
|
||
| /// Dominant Thirteenth Suspended Fourth Flat Ninth: Perfect Fourth, Perfect Fifth, Minor Seventh, Minor Ninth, Perfect Eleventh, Major Thirteenth, e.g. `C13sus4(♭9)` | ||
| case dom13_sus4_flat9 | ||
| case dom13_sus4_flat9 = "dom13_sus4_flat9" |
There was a problem hiding this comment.
Identifier Name Violation: Enum element name should only contain alphanumeric characters: 'dom13_sus4_flat9' (identifier_name)
Redundant String Enum Value Violation: String enum values can be omitted when they are equal to the enumcase name. (redundant_string_enum_value)
|
I like this better than tying to an Int. |
There was a problem hiding this comment.
Small but effective addition that enhances the robustness of the Framework 👏
I would like to discuss the broader concept of making Chords Codable and
maybe we can tackle this already in this PR 🤔
Chords include:
roottypeinversion
The root is a NoteClass, comprising of letter -> C, D, E, F, G, A, B
and accidental -> flat, sharp, double flat, and double sharp -> with appropriate raw values already in place.
The type ChordType is represented by a basic enum -> covered in your PR,
and inversion is simply an Integer so we are also good here I would say 👍
Should we consider establishing explicit raw values for the Letter Type also already in this PR? This way, if someone opts to convert them to lowercase in the future, it won't result in any issues when trying to decode previously persisted chords.
Or should we just keep the current raw values that are inferred by the compiler (C, D, E etc - uppercased)?
Wdyt?
The letter type idea is fine we could do it in another PR though |
|
At the end of the day, I see 3 options:
explicit Integer raw values: explicit String raw values implicit String raw values IMO, the least bad option is explicit integer and the worst option is implicit string. I know it's an eye sore to add the explicit raw values, but it feels like a safer approach to managing a core / model package. |
|
I think I'll avoid the raw values entirely for persistence purposes and instead It might add some overhead but in that case I can be sure that the package implementation won't break my persistence layer. So I don't mind anymore what values are used it. I am fine with Integers, String or even keeping it up to the User of the Lib. |
Good point - that's probably going to be my strategy as well. Otherwise it's a scary custom decoding path to walk 😅 |
By adding these explicit raw values, we are free to continue to adjust the cases (naming / spelling / fix any typos) without causing any breaking changes. We just can't change the raw values.
Alternatively, if we don't care what the raw values are (if nothing relies on the String value) - we could just pin each ChordType to an Int instead