HolyCityAudio / SpinCAD-Designer

SpinCAD Designer is an open source Java project which allows creation of patches for the Spin FV-1 audio DSP chip.
153 stars 27 forks source link

Compare input parameter #4

Closed luisfcorreia closed 6 years ago

luisfcorreia commented 6 years ago

Instead of comparing internal variable

(sorry, this will be the correct PR without experimental stuff)

Digital-Larry commented 6 years ago

Please give me more background about the reason for this change. The code under ElmGen was originally written by Andrew Kilpatrick and I'm not recently familiar with any of it. What problem does this solve?

luisfcorreia commented 6 years ago

The "final int freq" variable might not have been initialized at this time (Netbeans 8.2 told me this). This is perhaps a copy/paste error from the method right above it -> public LoadSinLFO(int lfo, double freq, double amp)

Digital-Larry commented 6 years ago

None of the final variables in this module are initialized:

final int lfo;
final int freq;
final int amp;

I have been using Eclipse Neon, I think. Notice there are local variables in this module with the same name as parameters passed in, and some of them are directly assigned those parameters and some have some transformation applied first. I do not believe your solution to be a good one to THAT problem (possibly uninitialized).

If you want to initialize those variables, then try something like:

final int lfo = 0;
final int freq = 0;
final int amp = 0;

I don't know if this will work, please try it.

luisfcorreia commented 6 years ago

In Java, final variables cannot be initialized but they can be modified. In my very humble opinion, in my change, what's checked for range shoud be the input parameter.

because in line 80 we do in fact change "this.freq" to the passed value.

the current comparison is just wrong.

I'll submit a pull request to the original code as well (on gitlab)

Digital-Larry commented 6 years ago

OK I see what you mean, I was looking at the wrong lines of code. Like I may have implied, I probably looked at this 3 years ago if that recently. Thanks!

Digital-Larry commented 6 years ago

Congratulations on submitting the first pull request ever on this project!