Very nice, and I really like your “statement of purpose”. As someone who programs c++ for my “day job”, please allow me to comment on the code…
First - the good stuff: the quantizer code has comments in it that are not only better than what I usually see in VCV code, but better than what I see every day in “commercial” code. Super useful for anyone who wanted to use this code.
Also - it’s very clearly written. It’s very readable. Again, much better than what I’m used to seeing. and, fwiw, on both of these points my code is pretty good, but yours is better.
Now - let’s tear it apart!
First thing I noticed is that when I view the code from the GitHub UI, it’s not lined up / indented correctly. I assume it looks fine in Visual Studio Code where you are writing it. This is almost always caused by putting tabs in your code. Github probably interprets tabs differently that your current settings in VSCode. While programmers argue about this all the time, I think the majority would agree that your code should use four spaces for indenting, rather than using tabs. You can easily re-configure VSCocde to do this.
Second thing: all of the functions in your quantizer are at “global scope” / “top level name-space”. Meaning that now you have a function called “noteName”. You can never have another function called noteName or they will conflict. Different people use different techniques to avoid this. Without getting overly detailed, you can put all this in a class, or in an namespace.
It would be very easy to retrofit a namespace. Just add namespace quantizer {
at the start and put a closing brace at the end. Then from you code call it as quantizer::noteName
. You an even avoid that if you say using namespace quantizer
at the top of the consuming file (but don’t do that).
Next: by passing around std::array you may be making copies of the arrays, which might be incurring a runtime overhead (make your plugin use too much CPU). The compiler may be smart enough to realize it doesn’t need to do this, but if you look in the docs, std::array<T>::operator =
is documented as making a copy. Easy work around, pass in a reference, like inline float quantize(float voltage, std::array<bool, 12>& validNotes) {
. For extra credit you can join the cool kids and use const std::array<bool, 12>& validNotes
).
Lastly, and this is really not important, your quantize algorithm needs to compare against possibly 12 pitches in the scale, which takes a bit of CPU time. Probably not important. The university people would say this algorithm is O(n), where n is the number of notes in the scale. If you instead used an std::set it would be O(log(n)) and std::unordered_set would be O(1).
But I’m just mentioning this to show off. 12 times though the loop is just fine, and doing it the cool way would be a lot of work and aggravation that you really don’t need.
Anyway, all these criticisms are kind of nit-picky and overly technical. That fact that the code is readable and commented means that next year you will probably be able to go back and understand it, and re-use it. I believe it also make for reliability – if you can look at it and see what it does it’s less likely there are bugs hiding in there.