# When is a number a magic number?

Over the last couple of months I have been diving into coding standard IfSQ. As a part of this IfSQ standard, a rule is to not use `Magic Numbers`. While I don't have a problem with building this rule into checks as FxCop or StyleCop, I am confused as to what actually IS a Magic Number.

According to Wikipedia there are 3 explanations of magic numbers:

1. A constant numerical or text value used to identify a file format or protocol;
2. Distinctive unique values that are unlikely to be mistaken for other meanings (e.g., Globally Unique Identifiers);
3. Unique values with unexplained meaning or multiple occurrences which could (preferably) be replaced with named constants.

I am focussing on the third meaning. But this is also where I am stuck. Seeing as I am trying to build this in a FxCop rule; the check has to be automated. But what am I supposed flag as magic number?

The IfSQ standard explains the following as magic numbers: `Numeric literals (other than 0 or 1) have been embedded directly into the source code. For example "34" or "86400".`

But this doesn't sound good to me at all. This would mean I'd have to flag every single occurence of a number other than 0 and 1. Numbers like 24 (hours), 60 (minutes) and 100 (percentage) are, in their correct context, no magic numbers; in my eyes that is.

Because this is will be an automated check, looking from things in a contextual way isn't really possible.

Based on this I have the following questions:

1. How should I define a magic number?
2. Is it possible to define magic numbers without context?
3. Should I skip certain statements when checking for magic numbers?
• Why are 24, 60 and 100 not magical to you ? Just because you know what they mean ? That criterion could be uses to ANY number since someone knows what any given number means. Stick to the canonical definition and don't make your own. Commented Jul 29, 2014 at 10:07
• @user61852: You are correct on that. But how would I distinguish non-magic numbers and magic numbers then? Seeing as any number could mean something to someone. If you were a computer, what would you describe as a magic number? From a purely objective point of view? Commented Jul 29, 2014 at 10:09
• Easy: any literal number other than 1 and 0 which is not part of a constant declaration. Commented Jul 29, 2014 at 10:11
• But what about this: `constant int x = 25 + 5`? You are declaring a constant, but with the use of two numbers. And how about this: `constant int y = x + 5`. Commented Jul 29, 2014 at 10:12
• Those are not magic numbers according to the definition, because those are constant declarations. If you get fastidios about it you will need to provide your script with an artificial intelligence similar to that of C3PO or Skynet in order to know every context of every human or alien culture. Is that the scope of your work ? Such a script will eventually become self-aware and revel agains humans. Please don't do that. Commented Jul 29, 2014 at 10:23

Just to extend a bit on Frank's answer, take the following:

``````private final int NUMBER_OF_HOURS_PER_DAY = 24;
private final int FRAMES_PER_SECOND = 24;
...
return NUMBER_OF_HOURS_PER_DAY * days;
...
return FRAMES_PER_SECOND * seconds;
``````

Now compare that to:

``````return 24 * days;
...
return 24 * seconds;
``````

If you consider that one of the most important aspect of code quality is how it conveys intentions, then the later of the above is a disaster.

• @Vatine My main point is that in programming numbers are used without units of measurements. One important aspect of physics is the calculus of quantities, which tells you for example that `hours/day * days = hours`, and that `frames/second * seconds = frames`. This aspect is almost always left out of arithmetic in code, and only conveyed through variable names. The intention above is that one `24` is not equal to the other, because they have different units, though implicit. Commented Jul 29, 2014 at 14:57
• @DaniellDinnyes Exactly. It would have been much easier to read if I'd used "HoursPerWeek" instead of 168 (which is the right number, but not a constant most people recognize on sight). Commented Jul 29, 2014 at 15:59
• Either they will be invaders from earth, in which case I must not chuck UTC (earth standard time...), however much a misfit it might be, or the sub-divisions we make would probably be useless, so I would have to do a complete re-write anyway. Just as serious as you here ;-) Commented Dec 18, 2014 at 10:09
• @Deduplicator, I think you missed the point of Daniel's answer. It's obvious that there are 24 "hours per day", but it's not obvious if that's the concept you are referring to when there are other "24" constants floating around in your code. Commented Dec 18, 2014 at 10:43
• @Deduplicator, sure, but while I do think that yours is a valid point, it is more an answer to the general "raw number vs. named constant" debate, it doesn't seem to address the concern that Daniel raised here. hours_per_week = 24 * 7 is fine, but frames_per_duration = 24 * 60 * 60 is already a bit more confusing... are we computing seconds per day or frames per hour? Sure, you might argue that the variable is named incorrectly, still naming the constants would help as well. Commented Dec 19, 2014 at 16:27

Most tools I know of, which support detection and reporting of magic number usages specifically only address statements. You are quite correct that eliminating all numbers from the source code is kind of pointless.

Who - with a sane mind - would read the number of hours a day has from a configuration file? Of course, there is a good reason to just use the 24, however there is no good reason to use it within a statement that is not part of a constant declaration. Compare the following two occurrences of 24:

``````private final int NUMBER_OF_HOURS_PER_DAY = 24;
...
return NUMBER_OF_HOURS_PER_DAY * days;
``````

versus:

``````return 24 * days;
``````

The second example above would typically be flagged as using a magic number, whereas the first one would not. The main difference comes from the fact, that a second usage within the code makes the difference clear: in the first example, you would simply refer to `NUMBER_OF_HOURS_PER_DAY` again and should your constant change at any time, you are sure that it's updated everywhere at once. In the second example you would just write `24` again, which increases the effect of the magic number problem.

Additionally, the constants can be named appropriately, so you get more meaning from it than from a simple magic number, which may not appear magical to you (because you know that a day has 24 hours), but may appear indeed quite magical to someone else. What if the factor was not 24, but 7? Is that any less magical? and does it even have something to do with the number of days in a week?

Either way, the net effect is that numbers (other than 0 and 1) should be defined via constants and you should not flag those occurrences as magic numbers. Numbers within statements, that are not used at a constant declaration site, should be flagged though.

Border cases: What to do with something like `final int HOURS = 20 + 4;`? and what about `final int CONSTANT = OTHER_CONSTANT * 4` ? Should the `4` be flagged as magic number or not? In my experience, the different tools differ at least in these border cases. Some are very strict and do not allow numbers in expressions that are larger than the constant itself (i.e., no addition/multiplication/etc.). Others are a bit more relaxed and accept it as a usage for defining the constant.

• The way I have my technology now, is that with `hours = 20 + 4`; both 20 and 4 are flagged as magic numbers. Looking at this from a contextual point of view, I can see these should not be flagged. But there is my problem again of not really being able to look at it from that point of view. What would you recommend from a non-contextual point of view? Should I skip over all constant-assignments? Commented Jul 29, 2014 at 9:30
• If your standard is clear, then you have to adhere to it. Another constant defined as `foo = 20 * 2` may or may not be a multiple occurrence of the `20`. If unsure, better report a false positive, than missing something. At least that's the rule of thumb for verification tools. Commented Jul 29, 2014 at 9:34
• While the standard is clear, I do not agree with the standard on this point. I guess rules are there to be broken, in this case? The problem with this is that, if I let through too many false-positives, the amount of false-positives on 1000+ lines of code would really get out of hand. I am looking for a line to draw as to when to flag it or not. The standard has drawn a line way too high (or low, depending on your viewpoint), for me. Commented Jul 29, 2014 at 9:39
• Make it a configurable behavior then. Your tool is not free to ignore the standard at this point, just because you do not agree. Others may have to depend on the tool to be 100% accurate to what the standard requires, no matter if they agree with that standard or not! Commented Jul 29, 2014 at 11:04
• There might be a small misunderstanding here, I am not developing a commercial tool of some sort. We have decided to include the IfSQ-standard in our companies standards; but with a few changes here and there. We have done the magic-numberdiscussion ourselves but couldn't come to one solid conclusion, hence the question here. The FxCop-rule (a code analysismechanism) will only be used internally. Commented Jul 29, 2014 at 11:08

I've struggled with this definition as well, in my case with CheckStyle.

A particular issue is in the Spring validation framework, where error strings are looked up in a .properties file. CheckStyle complains when I write something like

If this is the only place that string is used, I struggle to see what the alternative adds:

• The value is most obvious on the second usage. Consider someone wanting to do something like `hasError("error.invalid.email.address")`. Not DRY at all. Commented Jul 29, 2014 at 9:29
• I wonder if there is already a tool for generating the Java constants source file given the `".property"` file as input. For example, the generated constant file could contain `public final class Strings { public static final class Error { public static final class Email { public static final String Address = "error.invalid.email.address"; } } }` Commented Jul 29, 2014 at 10:17