Global Consts

Is this the way I should go? Are there any pit falls to doing this? Is it even worth it if its only like 3 - 5 constants?

Always store data (or define data) in ONE location. So, that's one way to do it. The pitfall to not doing it that way is 1) you have to edit several files to change anything, and 2) you may forget to edit one the files. Whether 1 constant or 100, wrong data will likely mess things up.

If there's a chance that the data may change during execution, for any object that needs the data, you can pass a pointer to a data structure or an instance of the class which maintains the data.

For constants that don't belong as part of an interface, yes, it can make a lot of sense to put them into one header file.

This is what they do with many parts of the standard library as well, such as the c++ header or the c header . The files are a dumping ground for a bunch of numeric constants in C, or a bunch of class wrappers for numeric constants in C++.

For constants that do belong to an interface, it usually makes the most sense to put the constants in the same header that contains the class or central namespace details.

The number of vertices per quad? That constant has a standard name, which is `4'. :)

Namespaces are your friend.

The number of vertices per quad? That constant has a standard name, which is `4'.

Unfortunately "4" can also be misread as the number of wheels on a shopping cart, so that's no good.

Better to just go with the unique name "NumVertsInQuad" and avoid readability issues

(in other words "magic numbers" are the enemy of good code, avoid them)

The number of vertices per quad? That constant has a standard name, which is `4'.

Unfortunately "4" can also be misread as the number of wheels on a shopping cart, so that's no good.

Better to just go with the unique name "NumVertsInQuad" and avoid readability issues

(in other words "magic numbers" are the enemy of good code, avoid them)

Now I wouldn't be surprised to see someone make the variable NumVertsInQuadOrWheelsOnShoppingCart.

const int four = 4;

const int four = 4;

Then in 5 years, the new guy on the team asks you why this code exists

const int four = 8; // must be 6 to fix render crash bug 

Seriously though, if it's a mathematical constant, like the number of corners that a rectangle has, or the number of faces of a cuboid, then there's no hurt in using the "magic" number as a literal number.

If there's some magic behind the number -- e.g. maybe a UI quad is actually two triangles, or 6 vertices, then you should definitely give that value a name instead of hard-coding it everywhere. Especially if there's a chance that the value might ever need to change -- e.g. on some platforms maybe a UI quad is actually a quad... or maybe you want to trim the invisible pixels by allowing them to be distorted octagons...

Unfortunately "4" can also be misread as the number of wheels on a shopping cart, so that's no good.

Better to just go with the unique name "NumVertsInQuad" and avoid readability issues

(in other words "magic numbers" are the enemy of good code, avoid them)

The thing is, there's no "magic" in this number. If you're drawing quads you expect to see the number 4, or a multiple of 4, and by having it in-place you immediately get to do a visual/mental sanity-check on the code rather than having to go scoot off to a header file somewhere else. In this case 4 is not a magic number.

The thing is, there's no "magic" in this number. If you're drawing quads you expect to see the number 4, or a multiple of 4, and by having it in-place you immediately get to do a visual/mental sanity-check on the code rather than having to go scoot off to a header file somewhere else. In this case 4 is not a magic number.

There's a related discussion over at http://programmers.stackexchange.com/questions/266717/are-all-magic-numbers-created-the-same which I'm sure many of you will have already read... and personally I agree. Normally only a small part of your program needs to know that a quad has four vertices, and I'm pretty sure that number isn't going to change without the "quad" part changing as well. But if naming it VERTICES_PER_QUAD or whatever the case may be gives you warm fuzzies...

It's all about readability to me.

I don't make a NumVertsInQuad const because I expect it to change (though yes, this does make it easier to change later), I make one so I can understand the code better.

numQuads = numVerts / 4;

vs.

numQuads = numVerts / NumVertsInQuad;

The second is quicker for me to understand. Sure, given a couple of seconds I could read the first example and figure out what "4" means, but in the second one I don't have to.

"4" is a "magic" number because on it's own it doesn't convey enough information. Its reason for existing might as well be magic until I read the context and figure out what it stands for.

It's all about readability to me.

I don't make a NumVertsInQuad const because I expect it to change (though yes, this does make it easier to change later), I make one so I can understand the code better.

Wouldn't the fact that this is (presumably) used in places that have to do with vertices that will be used to draw quads make this immediately obvious?

This discussion of whether the value of a constant is better to use than its meaning for clearest expression in code does not take in to account that 80% of the lifetime of a piece of code spent in maintenance mode.

Sure, you can show everyone how you're a Clever Dick because you know at this very moment that the context of a few characters out of millions of lines of code implies that a particular use of the value "4" implies it is the number of vertexes in a quad when rendering a particular set of vertexes using OpenGL, or the number of vertexes in a triangle fan pair in the GL|ES codepath. Later, the poor desperate engineer (maybe even you) who comes along to analyze why the clipping on a face is off and the boss is screaming in the office next door because ship dates are getting delayed, well, she'll just think you're a dick.

You're writing literate code for people to read and understand with minimal context. You're not writing for a compiler to understand, and you're not writing to show everyone how big your ego is (they already know, trust me). Those requirements, rarely met by most coders I've ever met, mean you should try to convey meaning (as in, use a named constant like vertexes_per_quad) rather than value (as in, 4).

I once found this in some code:

#define MINIMUM_POSITIVE_INTEGER 1

It was accompanied by the appropriate use of MINIMUM_POSITIVE_INTEGER whenever one needed to account for an off-by-one value.

I was not pleased.

It's all about readability to me.

I don't make a NumVertsInQuad const because I expect it to change (though yes, this does make it easier to change later), I make one so I can understand the code better.

Wouldn't the fact that this is (presumably) used in places that have to do with vertices that will be used to draw quads make this immediately obvious?

Maybe, maybe not. Making it a named const means I don't have to spend time figuring out what it means from context, especially if I'm unfamiliar with the code.

Personally I use constants with appropriate prefixes, in enums within namespaces:

namespace Constants
{
  const UINT VertsInQuad = 4;
  enum MetaSyntacticVariableNames
  {
    MSV_FOO = 1,
    MSV_BAR = 2,
    MSV_BAZ = 3,
    MSV_QUX = 4
  };
};

This discussion of whether the value of a constant is better to use than its meaning for clearest expression in code does not take in to account that 80% of the lifetime of a piece of code spent in maintenance mode.

SmkViper made an argument based on readability, proposing the number is actually more readable in some cases... You can't say the discussion isn't accounting for something while not reading the discussion.

Sure, you can show everyone how you're a Clever Dick because you know at this very moment that the context of a few characters out of millions of lines of code implies that a particular use of the value "4" implies it is the number of vertexes in a quad when rendering a particular set of vertexes using OpenGL, or the number of vertexes in a triangle fan pair in the GL|ES codepath. Later, the poor desperate engineer (maybe even you) who comes along to analyze why the clipping on a face is off and the boss is screaming in t

You're writing literate code for people to read and understand with minimal context. You're not writing for a compiler to understand, and you're not writing to show everyone how big your ego is (they already know, trust me).

You can make the readability argument without resorting to ad hominem -- that anyone who disagrees with you must simply be an egomaniac

Let's join in though, and make just as an insulting argument, rather than a productive one....
I could take SmkViper's code, which is perfectly readable, as the two nouns in the line already tell you the meaning of the magic 4:
numQuads = numVerts / 4;
Solve for 4:
4 = numVerts / numQuads ... or there are 4 verts per quad
If that's not obvious, we can add a comment:
numQuads = numVerts / 4; // there are 4 verts in a quad

That's readable. But sure, you can go ahead and fluff up your ego by showing everyone what a great overengineer you are with some more maintainabley extensiable codes:

IUnitConverter vertexCountToQuadCount =
      (new UnitConverterFactory(CurrentLocale))
           .MakeConverter(FromUnit<Vertices>::TypeValue, 
                          ToUnit<Quads>::TypeValue);
numQuads = vertexCountToQuadCount.Convert(numVerts);

There, much more readible and maintainextensable!

p.s. yes, this is farce. Being insulting doesn't make an argument stronger - probably the opposite...


Back to serious discussion land, there's still pitfalls that pop up in practice when giving names to magic numbers.

e.g. say for example we've designed an optimized quad renderer, which relies on the assumption that there's only 4 vertices.
For illustration, let's say we've got a 256 colour pallete that the verts can be coloured with, and we've got 4 verts. We can represent our vert colours with a single 32bit integer, by packing the four colour indices into the 4 bytes of that variable. On the GPU side we're limited to 4-byte aligned variables, but we've got dedicated instructions that can simultanously shift-and-mask to very efficiently decode this format.
Great! we've now got an optimal way to pack our quad corner colours into a vertex buffer.

Designing a nice generic system to come up with the optimal data packing format for every possible combination of values is a much larger tasks, and You Ain't Gonna Need It, so for now you just assume that the values are going to be what they currently are for the life of the project, and you complete your optimization task within those set parameters.

...But later, someone edits that magic named constant to be something other than 4... which causes all your code to be broken...


To deal with this, when writing this kind of optimized code, where good theory intersects with the practical facts of the hardware, you should always document every single assumption with assertions.
In this example, we can use a static assertion on the named variable, documenting that this code is only valid if the value is 4:

static_assert( NUM_VERTS_PER_QUAD == 4, "If this assertion fails, re-write the optimized colour packing routines" );

So then in the future, when someone changes that variable, instead of the code being buggy, it instead fails to compile and spits out a nice task description to add to your todo list.

Or you can take it a step further and turn that 4 into another named variable...

static_assert( NUM_VERTS_PER_QUAD == NUM_COLOURS_STORED_PER_QUAD, "If this assertion fails, re-write the optimized colour packing routines" );

But then later you'd probably have:

struct GpuBufferedQuad
{...
u32 packed_color_indices; static_assert( NUM_COLOURS_STORED_PER_QUAD * BITS_PER_COLOR_INDEX <= 32, "Assuming all the indices can be packed into a u32" );

Yeaaaaaaaaaaaah...

Anyway, if a constant relates to a specific class, make it a static const in the class. That's always handy because it's available directly within its own context and available publicly the same way it would be through a namespace:

class Foo {
public:
  static const int BAR = 42;
}

Foo::BAR; //is 42

Also, setting aside readability, avoiding magic numbers means that if you have to change the value at some point then you don't have to do the easter-egg hunt for all the relevant replacement positions. Even for trivial cases this can make sense:

const int ARY_SZ = 10;
int ary[ARY_SZ];
for(int i = 0; i < ARY_SZ; i++) {
  baz(ary[i]);
}

If you change the size of the array then you only have to change it in one place. In more complex code this can save a lot of heartache.