midi2-dev / AM_MIDI2.0Lib

A MIDI 2.0 C++ Library
MIT License
23 stars 2 forks source link

Add a FIFO class and use it to replace the code in bytestreamToUMP. #13

Closed paulhuggett closed 4 days ago

paulhuggett commented 3 months ago

Whilst reading through the bytestreamToUMP code I realized that the readIndex, writeIndex, bufferLength, and umpMessage fields are used together to form a FIFO/circular buffer. I've added a FIFO class to util.h and replaced those fields with an instance. The separates the responsibility of maintaining the FIFO and simplifies the more interesting code!

This new class also reduces overhead for a 4 element container to 1 byte vs. 3 integers previously and has the further benefit that is guards against under/overflow abuses.

This commit only affects the FIFO in this one header file. There is another in umpToMIDI1Protocol.cpp which should probably be switched over. I haven't done that yet because I want to limit the scope of this change.

That only leaves the quesiton of how to unit-test the code... For this kind of thing I usually use google test. Do you have a preference for how this is done? What would you think about adding it as an optional dependency?