-5

Why are magic NUMBERS considered bad practice? For example:

memcpy(ResourcesDir+GameDirLen, "/Resources", 11);

What is wrong with the 11? Why are magical numbers so bad?

Please, my question is entirely different from this one. Mine is about why they're considered bad, and the linked one is about what qualifies as one.

  • 5
    Does this answer your question? Is every number in the code considered a "magic number"? – gnat Jun 8 at 20:10
  • @gnat No, why are they bad? Not are all NOMBERS considered them? – user367691 Jun 8 at 20:11
  • 4
    Well, the answer to this question is, definitely, 42. – Vector Zita Jun 8 at 20:24
  • 1
    Does this answer your question? Usage of magic strings/numbers – Doc Brown Jun 8 at 20:47
  • 4
    You are making an assumption here that people know what you mean by a "magic number" so it would be better to describe what a magic number is as far as you are concerned. The act of describing the "magic number" might lead you to a better understanding of why these numbers are problematic. In short, they have no meaning except to the person who was writing the code and only as long as they remember that meaning. – Jason K. Jun 9 at 0:19
5

Let's start by defining a magic number.

3.14

What is that? Is it pi? If it is, it should be codified into your program as:

const double pi = 3.14;

Why?

  1. The name gives it meaning beyond it being just some arbitrary number.
  2. It's more readable.

    double area = pi * radius * radius;
    
  3. You can change it in one place (for example, adding more digits of precision), and it will change throughout your entire program.

OK, since this answer seems to be getting a lot of negative attention, let's talk about enums.

Which is easier to read and easier to maintain... This?

if (trafficLightColor == 1)

or this?

if (trafficLightColor == Color.Red)
| improve this answer | |
  • Why not #define PI ((double)3.14) – user367691 Jun 8 at 20:17
  • That works too. Sounds like you know more about this than you let on. – Robert Harvey Jun 8 at 20:18
  • +1 because PI is really magic. And avoid sqrt(2) in public examples because of the Pythagorians ;-) – Christophe Jun 8 at 21:17
  • Please don't define Pi with only 3 significant digits. – whatsisname Jun 9 at 17:47
  • 1
    @whatsisname: Precision is relative to how precise you want (or can be bothered) to be. 3 significant digits is perfectly fine for a casual approximation. Also, your suggestion to not use 3 leads one to wonder which amount would satisfy you (and others in general), which amuses me even more as we'd be deciding on a magical number (of significant digits) in a question on why to avoid magical numbers. – Flater Jun 18 at 10:42
4

In short

It's a bad practice because "magic numbers" make localization and maintenance more difficult, since someone else (or a future version of you) will not know (anymore) why this magic number was as it was.

So the good practice is to avoid magic numbers. Or at least, give them a name with const or #define, and some comments to remind how they are determined. And if possible, isolate them in the code: in case of change, people will know where to look and how to change them.

What it means in practice

Your simple example requires some imagination to demonstrate the issue: Suppose the new art director of your game company decides that all the resources should from now on considered as assets. Someone might do a search/replace that will result in:

memcpy(ResourcesDir+GameDirLen, "/GameAssets", 11);    // OUCH!!!

Or maybe someone decides that resources are fun and adds some smiley (UTF8 encoded of course):

memcpy(ResourcesDir+GameDirLen, "/Ress🙂urces", 11);   // OUCH!!! x3 
                                         // (because of 4 bytes encoding instead of 1) 

Everything will compile. But in the first case you've lost your trailing '\0', which might cause buffer overflows. And in the second case, the buffer overflow is already there: your new release will ruin all the past success of your game because of security considerations and bad quality.

Your example is about a directories behind the scene. But now imagine that it's about dialogues and messages that have to be translated in several languages, each using a translation of different length...

Terminological remark

The term "magic number" has multiple meanings :

  • I handled it here in the sense of "a unique unexplained constant value", and not in the other common meaning of a special integer at the beginning of a file to give a hint about the file's content.
  • As there are some fierce battle about what a magic number is, it may be worth mentioning that some well known secure coding standards and organisations use the term likewise.
| improve this answer | |
1

A plain number like 11 can have different meanings - it could mean “eleven things”, or it could be an error code like “thing not found”, or it could be a command code to “do this thing”, or it could be a bitmask to check several flags at once, and it can mean all of those things in the same code.

This makes maintenance a nightmare, because if you need to start counting “twelve things” but leave the other instances of 11 alone, then you can’t do a simple search and replace, you have to manually check each instance of 11.

If you ever have a situation where 11 means anything other than “eleven things”, then you are better off creating a symbolic constant for it (preprocessor macro, enumeration constant, or a const-qualified variable). The code will be easier to understand and maintain.

And to echo Andrew’s comment, make those names meaningful. If 11 is a code for a specific action, name it for that action:

#define THIS_ACTION 11
#define THAT_ACTION 12
...

If it’s an error code, name it for the error:

#define NOT_FOUND 11

etc.

| improve this answer | |
  • 1
    Also, when naming your variables, name them with some meaning: don't use ElevenThings = 11 because it can confuse maintainers even more if you need to extend... I still have nightmares over a snippet I once came across something akin to: ElevenThings = 12 /* Actually we need 12 things */ – Andrew Jun 14 at 14:43
1

There are two bad things with magic numbers, one of which is intrinsic to the concept and one that just is a consequence of practicalities.

The intrinsic drawback is that it's so much harder to understand. It does not convey any information except for the raw value. A named constant is much more readable.

The practical drawback is that whenever a magical number occurs in code, it's very common that it occurs at multiple places. So if you want to change it, you need to change it at several places. It's essentially code duplication. Example:

const int no_iterations = 1000;
int sum = 0;
for(int i=0; i<no_iterations; i++) {
    printf("Executing iteration %d of %d\n", i, no_iterations);
    sum += foo();
}
printf("Average value: %f\n", (double)sum/no_iterations);

Without named constants, you would have to change three occurances, which can be easy to miss. Maybe you think you could just do a search and replace for the value? Nope. Consider this code:

double sphere_volume(double r) { return r*r*r*3*pi/4; }

int main() 
{
    double spheres[4];
    // Yes, that's how to figure out the number of elemens in an array in C
    for(int i=0; i<sizeof(spheres)/sizeof(spheres[0]); i++)
        printf("Volume of sphere %f: %f\n", sphere_volume(sphere[i]);
}

Suppose you refactor the code by replacing all 4 with NO_ITER and define a #define NO_ITER 4. The code will work as before, until someone else decides that we now should change the number of iterations.

In our particular example, the code duplication is hidden. If you change the string to something else, you must ALSO remember to change the last argument. A better solution is this:

const char str[] = "/Resources";
memcpy(ResourcesDir+GameDirLen, str, sizeof(str));
| improve this answer | |