Eliminating Magic Numbers: When is it time to say "No"?

Question

We're all aware that magic numbers (hard-coded values) can wreak havoc in your program, especially when it's time to modify a section of code that has no comments, but where do you draw the line?

For instance, if you have a function that calculates the number of seconds between two days, do you replace

seconds = num_days * 24 * 60 * 60

with

seconds = num_days * HOURS_PER_DAY * MINUTES_PER_HOUR * SECONDS_PER_MINUTE

At what point do you decide that it is completely obvious what the hard-coded value means and leave it alone?

Answer 1

There are two reasons to use symbolic constants instead of numeric literals:

1. To simplify maintenance if the magic numbers change. This does not apply to your example. It is extremely unlikely that the number of seconds in an hour, or the number of hours in a day will change.

2. To improve readibility. The expression "24*60*60" is pretty obvious to almost everyone. "SECONDS_PER_DAY" is too, but if you are hunting a bug, you may have to go check that SECONDS_PER_DAY was defined correctly. There is value in brevity.

For magic numbers that appear exactly once, and are independent of the rest of the program, deciding whether to create a symbol for that number is a matter of taste. If there is any doubt, go ahead and create a symbol.

Do not do this:

public static final int THREE = 3;

Answer 2

I'd keep the rule of never having magic numbers.

While

seconds = num_days * 24 * 60 * 60

Is perfectly readable most of the time, after having coded for 10 hours a day for three or four weeks in crunch mode

seconds = num_days * HOURS_PER_DAY * MINUTES_PER_HOUR * SECONDS_PER_MINUTE

is much easier to read.

FrustratedWithFormsDesigner's suggestion is better:

seconds = num_days * DAYS_TO_SECOND_FACTOR

or even better

seconds = CONVERT_DAYS_TO_SECONDS(num_days)

Things stop being obvious when you're very tired. Code defensively.

Answer 3

One of the best examples I have found for promoting use of constants for obvious thing like HOURS_PER_DAY is:

We were calculating how long things were sitting in a person's job queue. The requirements were loosely defined and the programmer hard coded 24 in a number of places. Eventually we realized that it wasn't fair to punish the users for sitting on a problem for 24 hours when the really only work for 8 hours a day. When the task came to fix this AND see what other reports might have the same problem it was pretty difficult to grep/search through the code for 24 would have been much easier to grep/search for HOURS_PER_DAY

Answer 4

The time to say no is almost always. Times where I find it's easier to just use hard-coded numbers is in places such as UI layout - creating a constant for the positioning of every control on the form gets very cubmersone and tiring and if that code is usually handled by a UI designer it doesn't much matter. ...unless the UI is laid out dynamically, or uses relative positions to some anchor or is written by hand. In that case, I'd say it's better to define some meaningful constants for layout. And if you need a fudge factor here or there to align/position something "just right", that should also be defined.

But in your example, I do think that replacing 24 * 60 * 60 by DAYS_TO_SECONDS_FACTOR is better.

---

I concede that hard-coded values are also OK when the context and usage is completely clear. This, however, is a judgement call...

Example:

As @rmx pointed out, using 0 or 1 to check if a list is empty, or maybe in a loop's bounds is an example of a case where the purpose of the constant is very clear.

Answer 5

Stop when you can't pin down a meaning or purpose to the number.

seconds = num_days * HOURS_PER_DAY * MINUTES_PER_HOUR * SECONDS_PER_MINUTE

is much easier to read than just using the numbers. (Although it could be made more readable by having a single SECONDS_PER_DAY constant, but it's a completely separate issue.)

Assume that a developer looking at the code can see what it does. But do not assume that they also know why. If your constant helps understanding the why, go for it. If not, don't.

If you'd end up with too many constants, as was suggested by one answer, consider using an external configuration file instead, as having dozens of constants in a file doesn't exactly improve readability.

Answer 6

I'd probably say "no" to things like:

#define HTML_END_TAG "</html>"

And definitely would say "no" to:

#define QUADRATIC_DISCRIMINANT_COEF 4
#define QUADRATIC_DENOMINATOR_COEF  2

Answer 7

I think that so long as the number is completely constant and has no possibility of changing, it's perfectly acceptable. So in your case, seconds = num_days * 24 * 60 * 60 is fine (assuming of course that you don't do something silly like do this sort of calculation inside a loop) and arguably better for readability than seconds = num_days * HOURS_PER_DAY * MINUTES_PER_HOUR * SECONDS_PER_MINUTE.

It's when you do things like this that's bad:

lineOffset += 24; // 24 lines to a page

Even if you couldn't fit anymore lines on the page or even if you have no intentions of changing it, use a constant variable instead, because one day it's going to come back to haunt you. Ultimately, the point is readability, not saving 2 cycles of calculation on the CPU. This is no longer 1978 when precious bytes were squeezed for all their worth.

Answer 8

I would avoid creating constants (magic values) to convert a value from one unit to other. In case of converting I prefer a speaking method name. In this example this would be e.g. DayToSeconds(num_days) internal the method don't need magic values because, the meaning of "24" and "60" is clear.

In this case I'd never use seconds / minutes / hours. I would only use TimeSpan/DateTime.

Answer 9

seconds = num_days * 24 * 60 * 60

Is perfectly fine. These aren't really magic numbers as they will never change.

Any numbers that can reasonably change or have no obvious meaning should be put into variables. Which means pretty much all of them.

Answer 10

Use context as a parameter to decide

For example, you have a function called "calculateSecondsBetween: aDay and: anotherDay", you won't need to do much exaplanation about what those numbers do, because the function name is quite representative.

And another question is, which are the possibilities to calculate it in a different way? Sometimes there are lots of ways to do the same thing, so to guide future programmers and show them what method you used, defining constants could help figure it out.