Melody refactor

[juleskers]

I'm practicing my refactoring-fu, and have been wanting to try out JUnit5 for ages.
The result is this branch :smile:

Please let me know what you think!

It:

• moves song definitions to dedicated files.
  • I think this makes contribution of future songs more accessible, as the "template" for a new song becomes disentangled from app-logic.
• extracts all the String -> key_idx conversion done in Piano to a dedicated NoteMapper
  • as well as the octave-autocompletion that was hidden in Melody
• adds JUnit5 so that I could actually write tests for this
  • Google only supports JUnit4 natively in their gradle plugin, so I needed to add a community plugin:
    https://github.com/mannodermaus/android-junit5
    I'm not sure how acceptable that is?
• Convert Melody to store already-parsed key_idx, rather than String-notes.
  • This means notes are only bulk-parsed once during app-initialisation, rather than one-note-at-a-time (and repeatedly) as we hit keys.
• Another test to check that I didn't break song-parsing: iterates over all available songs to check they don't contain NO_NOTE(==5)
  • 
• Some polish and JavaDoc that I added as I worked my way through understanding Piano

[pserwylo]

These are great, thanks. I personally prefer the hashmap to the large case statement (am not sure that the space is something to worry about given even much older and low-spec devices), but am not fussy. Super happy with everything else, especially the test cases. I'll go and add a GitHub Action file to run those tests and do a test build after merging.

Regarding the Melodies in single files - I did contemplate moving them into XML resource definitions, perhaps Android "asset" files, which would allow us to scan them as easily as scanning a directory. The only thing that stopped me (other than time) was the internationalisation of song names. Right now, we are almost forced to provide a strings.xml entry for each song title. Whereas if it is in XML, then we'd have to do extra work to make sure that the strings.xml contains an entry for each corresponding file, and the compiler wouldn't check it for us. I think your solution of separate classes for now is a good middleground. We don't want to have 10,000 songs to choose from anyway!

[juleskers]

Glad you like it, @pserwylo . I admit I was a bit nervous so thoroughly taking apart your brainchild :sweat_smile:

• Github Action: very cool! I was looking into this as well, but I've never used either Github actions, nor gradle/android tests, so if you already know how to do this, I'll gladly watch a pro at work :blush:
• Song-definitions: Yeah, translation of those is fun. I'm already discovering that the existing songs have completely unrelated names in Dutch and German.
  • As for compile-time checking: we could always write androidDebug tests that scan for song defitions, and then try to fetch translation-strings for all languages.. I do something similar at $DAYJOB to test for duplicated error numbers across a 500KLoC project. Then again, failing our test suite because of missing translations virtually guarantees our tests fail 99% of the time.
• 10.000 songs: what, we don't?!? Guess I'll throw away that branch then :stuck_out_tongue_closed_eyes:
  But yeah, I do have a handful of songs I'd like to contribute, and I think classfiles are a good middle ground too. I actually like that we can javadoc the song for a bit of background, and am really happy with the Lyrics-comment + notes-String convention you started. I don't think that would work as well in XML resources.
  • expect "Are you sleeping, Brother John", which is known in almost every country in the world;
    German Wikipedia lists 52(!) translations, the Chinese translation is apparently about two tigers?