Best Practices for avoiding Magic Numbers


#1

I’m starting to get into some more complicated plug-in projects with JUCE, and working on keeping code as sane and readable as possible as the complexity grows.

One area that’s already a bit messy is the use of magic numbers in the Processor constructor, when setting up filters or other dsp object instances whose values won’t be changing during runtime.

For example, say I’m doing some kind of guitar amp sim, and for part of that there’s a bank of biquad filters for emulating the amp’s tone stack. Those frequencies won’t be changing during runtime. In the Processor’s constructor, I’ve got something like this:

biquad[0].init(sr, 4, 100, 0.8);
biquad[1].init(sr, 3, 1000, 1.2);
biquad[2].init(sr, 5, 5000, 0.7);

Now, if instead of using magic numbers in those init calls, the frequency and Q values could be replaced by constants of some kind. It would allow a quicker read of all the hardcoded parameters’ values in one place at the top of the Processor file, and it would allow a clearer read of the biquad init calls when viewing this constructor code. Yet I don’t see this done in the code examples I’ve looked at.

So, I’m hoping to open the “best practices” of this up for discussion here. But as one specific question: which type of constant do you think would be best for this case?

const float tonestackLowFreq = 100;

or

#define TONESTACK_LOW_FREQ 100

???

Besides the stylistic choice, there’s the difference of type checking when using a const float. But do you see that difference as more of an advantage, or an (unwelcome) constraint, in this context?


#2

Definitely no define. That is breaking most IDEs, and is not type safe until it is compiled.

What I tend to do, is putting it into a namespace and use static inline:

namespace ToneStack
{
    static inline float lowFreq = 100.0f;
    static inline float midFreq = 1000.0f;
    static inline float highFreq = 5000.0f;
};

biquad[0].init(sr, 4, ToneStack::lowFreq, 0.8);
biquad[1].init(sr, 3, ToneStack::midFreq, 1.2);
biquad[2].init(sr, 5, ToneStack::highFreq, 0.7);

And you can even keep that in the cpp if possible, so it is neatly contained.


#3

I usually go with something like static constexpr double kGainSmoothTime = 0.005; on top of the CPP file. This will resolve expressions at the compile time and enforces typing.


#4

what’s the k in that name for?


#5

Konstant? Germany rulez lol


#6

Konstant indeed … I think it is Apple that inspired me :slight_smile:


#7

Ditto!


#8

Ah, see, that’s why I’m glad I asked this question - I wouldn’t have thought of using namespaces!

As you might have inferred from seeing my choice of variable name, I try to organize variables through consistent naming standards - in this case, it would have been a consistent prefix:

const float tonestackLowFreq = 100;
const float tonestackMidFreq = 1000;
const float tonestackHighFreq = 5000; 

But I like how that use of the namespace in your example formalizes the hierarchy.


#9

I believe that K is for Kompressor…


#10

Actually, @daniel, speaking of formalizing the hierarchy, would you extend this approach to using nested namespaces?

namespace ToneStack
{
    namespace Low
    {
        static inline float Freq = 100.0;
        static inline float Q = 0.8;
    }
    namespace Mid
    {
        static inline float Freq = 1000.0;
        static inline float Q = 1.2;
    }
    namespace High
    {
        static inline float Freq = 5000.0;
        static inline float Q = 0.7;
    }  
};

#11

Well, you can certainly do that. I feel it’s a bit over the top.
If you want to govern the fact, that each filter coefficients have a certain structure, you could create a struct:

namespace ToneStack
{
    struct Coefficients
    {
        float freq = 0.0f;
        float q = 1.0f;
    };

    static inline Coefficients makeLow() { return { 100.0, 0.8 }; }
    static inline Coefficients makeMid() { return { 1000.0, 1.2 }; }
    static inline Coefficients makeHigh() { return { 5000.0, 0.7 }; }
};

const auto low = ToneStack::makeLow();
biquad[0].init(sr, 4, low.freq, low.q);

But like I said, I personally think that’s overkill, unless your init() can be refactored to accept the ToneStack::Coefficients struct as argument.


#12

Thank you for the example, this has been very helpful.

What is the advantage of using the makeLow method, rather than just doing this?

namespace ToneStack
{
    struct Coefficients
    {
        float freq = 0.0f;
        float q = 1.0f;
    };

    static inline Coefficients low = { 100.0, 0.8 };
    static inline Coefficients mid = { 1000.0, 1.2 };
    static inline Coefficients high = { 5000.0, 0.7 };
};

biquad[0].init(sr, 4, ToneStack::low.freq, ToneStack::low.q);

#13

I’m not a “C++ lawyer”, but IIRC inline variables are a C++17 feature. The syntax that @daniel used should only require C++11.


#14

Oh, right, that makes sense.