Rationalise `Tickable`, `Note` ...

[rvilarl]

In the current situation:

• Note is the only one class extending Tickable
• Most of the pending ts-nocheck after merging #1141 are related to this topic.

@0xfe @ronyeh Should we merge both classes in one?

[rvilarl]

Another possibility would be to extend Tickable with abstract methods required by the different classes using it in order to avoid casting to Note, StemmableNote and StaveNote.

[rvilarl]

@ronyeh you had already some thoughts on this one, I believe.

[0xfe]

Thanks @rvilarl. Merging them feels like a hack -- if we're going to explore this path, I'd suggest we think of how interfaces can help us here.

E.g., I think we can have separate Tickable and Note interfaces that more cleanly separate the intents.

Tickable is anything that consumes time in a voice. And Note is something that has a pitch (or group of pitches). In addition, we can have a Formattable for elements that can be formatted into a stave.

Elements like StaveNote, TabNote, GraceNote etc. implement these interfaces, and core VexFlow stuff like Voice and Formatter expect them.

Does that make sense?

[rvilarl]

I see the point, but:

• is there anything else than a Note or child class from Note that should be a Tickable?
• the classes using the tickables in Voice, expect them to be Notes.

[0xfe]

• Not right now, however that's beside the point, because Note has very low cohesion, and a very bad abstraction. Removing Tickable will make that worse.
• Right, I think that is bad too, because Note is just a catch all abstraction.

If we're going to refactor this, it should move in the direction of higher cohesion.

There's definitely a problem with the current abstractions, and you're right about that, but merging Tickable and Note makes it worse, IMO.

[rvilarl]

@ronyeh anything to add from your side? Otherwise I will close this issue.

[ronyeh]

:-) I'll defer to Mohit on this since he's the original designer.

But going by the class comments:

Tickable represents a element that sit on a score and has a duration, i.e., Tickables occupy space in the musical rendering dimension.

...
Notes have some common properties: All of them have a value (e.g., pitch, fret, etc.) and a duration (quarter, half, etc.)
...

So could we brainstorm up some things that sit on a score, but do not have a pitch or fret value? I guess technically a rest would be a Tickable but not a Note, but in VexFlow rests are a special type of note.

What if I wanted to notate a part of a score as a jazzy ad lib? Maybe that's a TextNote, but maybe TextNote should actually be TextTickable (or a different name), haha...

One idea might be if I wanted to make a sheet music quiz game, where I put big question marks (or a empty rectangle) into the score. The student would have to guess what notes go into the empty spot. This TextTickable or ShapeTickable would definitely take space on the score but not have an associated pitch.

Maybe future PRs can explore how we can separate other tickable types from Note (or whether we should do it at all).

[rvilarl]

I would recommend in case of doubt / redesign to follow MusicXML standard. This format is now accepted by many music editors. There we can challenge also if we have the required attributes for a notehead, gracenote,...

The rest in MusicXML is an atribute of a Note. Notes have exactly one of the following pitch, unpitched, rest https://www.w3.org/2021/06/musicxml40/musicxml-reference/elements/note/

The definition of voice is also interesting. The only parent elements (elements which can be in a voice) are direction, forward(vexflow figures this out in run-time) and note https://www.w3.org/2021/06/musicxml40/musicxml-reference/elements/voice/

[rvilarl]

The changes in #1182 were not useful and I would like to further discuss how to proceed. This is the hierarchy that we have today:

I would say that BarNote, ClefNote, Crescendo, KeySigNote, NoteHead, TextDynamics, TextNote, TimeSigNote are not Note, thoughts?