Skip to content

Conversation

@SLUCHABLUB
Copy link
Contributor

@SLUCHABLUB SLUCHABLUB commented Dec 5, 2025

I've implemented some of the things I had in mind. Some changes may be more "controversial" than others so I'm prepared to cherry pick the commits.

TO-DO list:

  • Run scala fmt
  • Remove Scala 2 syntax
  • Use scientific pitch notation (middle C is C4 not C5)
  • Remove some unneeded brackets.
  • import stuff
    There are a lot of places where objects are fully qualified. I think it would be more readable if they were imported.
  • Use math.floorMod when calculating relativePitchClasses
  • Use more standard music terminology.
    For example change "relative pitch classes" to "simple intervals".
  • Rework some names
    Some names are (in my opinion) unnecessarily compressed. Whilst I can accept the use of single letter names (at least outside of public API), I would like to change nbrOf* and to*Opt to numberOf* and to*Option to align with the naming in std.
  • Use enums in place of Ints
    There is a nauseatingly high usage of integers throughout the lab for things that should be enums. On one hand it could prompt students to refactor the api themselves. On the other hand we should encourage the use of enums and in my my experience most people are quite cautious when it comes to changing the given code.
  • Update the compendium
    The compendium needs to be updated to reflect (some of) the above changes.
  • Look over the scalafmt output and remove potential regressions in readability

@bjornregnell
Copy link
Member

Thank you for your work on this! Let's discuss on the lecture breaks the coming week.

I've chosen to import names at the file level unless other related names were imported in a tighter scope.
I did this to make function bodies smaller and since I felt it was waranted in most cases.
It is the more common name withing music theory for this.
One could call this "intervall classes" to be in line with "pitch classes" since they both are equivalence classes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants