music-notation-swift / music-notation-core

Music Notation Library in Swift (Deprecated. See `music-notation` instead)
MIT License
115 stars 16 forks source link

Implementing Ties #1

Closed drumnkyle closed 8 years ago

drumnkyle commented 9 years ago

Implement tying 2 notes together. The method is stubbed already.

Tasks to complete:

drumnkyle commented 8 years ago

I started out writing the API for adding and removing a Tie within Measure. However, I realized during development that you could have a case where a tie could start in one measure and end in another. Because we wouldn't want measures to know about each other, it makes sense to instead have this API in the Staff. Therefore, I now started implementing it in the Staff. I have not removed it from Measure yet, because I'm not sure if the Staff implementation will call into Measure or will just modify Measure itself yet.

drumnkyle commented 8 years ago

Just had to make a decision about how repeats affect measure indexes. This is what I have decided at this point:

If you have a repeat of a count of measures, each measure should be able to be indexed, so even if a measure is a repeat of a previous measure, it will have its own index. For instance:

[ MeasureA, MeasureB, MeasureRepeat(2 measures, repeated once), MeasureC ]

MeasureA is index 0 MeasureB is index 1 Measure0 of repeat is index 2 Measure1 of repeat is index 3 Measure0 of repeat next count is index 4 Measure1 of repeat next count is index 5 MeasureC is index 6

I think this makes sense, because it will allow every measure to be accessed even if it is part of a repeat. However, for modifying measures, the code will need to ensure that you never try to modify a measure that is simply a repeat of an earlier measure. I can see a possibility of maybe using a flag in Measure to indicate this for convenience possibly at some point.

drumnkyle commented 8 years ago

Considering having a state for a Measure that says whether it can be mutated, because if a measure is just a repeat of another one, then you shouldn't be able to modify anything in it.

drumnkyle commented 8 years ago

This branch is ready to merged and can be reviewed now. Every task is completed.

Take a look @migue48

drumnkyle commented 8 years ago

Since this is a very large change, which I will avoid in the future, we should try to do a code walkthrough before you take a look. Contact me to schedule it @migue48