Code review - Githubissues

Hi, you asked for someone to look over your code and I said I would.

It all looks good to me in terms of efficiency and process. I only have minor comments, one is inefficient but the rest are purely stylistic if you don't mind me mentioning them.

In the PatchSequencer::step function you evaluate this

(int) clamp(roundf(params[STEPS_PARAM].value), 1.0f, 16.0f);

three times. As far as I can see, you could do it just once, or you could use a SnapKnob to let the UI handle the rounding for you.

You have a huge initialization section in the PatchSequencer module. You could keep that simpler; in c++ an initializer with insufficient items to fill the array causes the compiler to initialize any remaining values to defaults. In the case of bools and floats that would be zero. so you could do:

bool switch_states[16][10][10] = {};

and that should be sufficient. If you don't provide any initializer at all, then the array will be uninitialized.

In the PatchSequencer enums you have a mix of idioms. Some have the ENUMS macro to account for multiple items, and some have a more explicit x = y + 10 sort of thing. Personally I would try to stick to one for clarity.
Again the PatchSequencer has a huge number of occurrences of 10 and 16 as 'Magic Numbers'. General good practice would be to factor these out. There at least 3 approaches.

Macros are generally simple, but these days frowned upon in c++ because their scope is uncontrolled.

Const declarations would work here, and can be scoped to the ComputerscarePatchSequencer class.

If there is a possibility that you might want to make another patch sequencer with different dimensions, then you could consider using a template class. This is what I have done in my modules. There is a simple example of a templated module at NG1.cpp. And a more complex example of a multi-dimensional device with templates for both the module and the moduleWidget at AO1.cpp

freddyz / computerscare-vcv-modules

Code review #2