search

ozankasikci / rust-music-theory

A music theory guide written in Rust.
MIT License
626 stars 28 forks source link

Clone, Display derives + PitchClass::into_u8 #6

Closed XBagon closed 4 years ago

XBagon commented 4 years ago

Added some missing Clone and Displays derives for easier library usage. Needed to clone Notes myself and added the other ones for completion.

Also added PitchClass::into_u8 for cases when you need the number representation, in my case for calculating the MIDI note number.

ozankasikci commented 4 years ago

Could you also add tests for the into_u8 method?

XBagon commented 4 years ago

Could you also add tests for the into_u8 method?

I would say it's pretty hard to write a meaningful test here, as the method only does the exact mapping you could test for. So yea, if you want I can write an test, that probably looks very similar to the function😄.

ozankasikci commented 4 years ago

Could you also add tests for the into_u8 method?

I would say it's pretty hard to write a meaningful test here, as the method only does the exact mapping you could test for. So yea, if you want I can write an test, that probably looks very similar to the function😄.

I get your point, it's not the most meaningful test, but still makes a contract with the current codebase. That makes sure no one can accidentally change the values without failing the tests. So there is still a reason to write the tests, would appreciate if you could do that :)