dn-m / MusicXML

Implementation of the musicXML specification in Swift
MIT License
74 stars 20 forks source link

Fix Key.NonTraditional decoding re: optional key-accidental values #125

Closed jsbean closed 5 years ago

jsbean commented 5 years ago

This fix presented #120 does not address cases like the one below. As per the spec, it would be possible to see something like this:

<key>
    <key-step>B</key-step>
    <key-alter>-1</key-alter>

    <key-step>E</key-step>
    <key-alter>-2</key-alter>
    <key-accidental>slash-flat</key-accidental>

    <key-step>A</key-step>
    <key-alter>-2</key-alter>
    <key-accidental>slash-flat</key-accidental>

    <key-step>F</key-step>
    <key-alter>2</key-alter>
</key>

With this fix, this example would not parse correctly.

@DJBen presented:

See my latest PR #124, I encountered the same issue with harmony having imploded group that is a list: harmony-chord.

I've tried many decoder combinations but none avail. My final solution is to parse them into a list of enums like below

enum KeyComponent {
 case keyStep(KeyStep)
 case keyAlter(KeyAlter)
 case keyAccidental(KeyAccidental)
}

and reassemble it into multiple Keys (see #124).

@bwetherfield adds:

That looks good! Would something like this be possible to enforce order?

while !valuesContainer.isAtEnd {
    do {
       keyComponents.append(.keyStep(try valuesContainer.decode(KeyStep.self)))
       keyComponents.append(.keyAlter(try valuesContainer.decode(KeyAlter.self)))
        keyComponents.append(.keyAccidental(try valuesContainer.decodeIfPresent(KeyAccidental.self)))
   } catch {
       break
   }
}

This would work with:

enum KeyComponent {
 case keyStep(KeyStep)
 case keyAlter(KeyAlter)
 case keyAccidental(KeyAccidental?)
}