37

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?

Improve this question
12
  • 2
    Why not replace that calculation with a function or macro so your code ends up looking like seconds = CALC_SECONDS(num_days);
    – FrustratedWithFormsDesigner
    Mar 9, 2011 at 16:31
  • 17
    TimeSpan.FromDays(numDays).Seconds;
    – Nobody
    Mar 9, 2011 at 16:34
  • 21
    @oosterwal: With that attitude (HOURS_PER_DAY will never need to be altered), you'll never be coding for software deployed on Mars. :P
    – FrustratedWithFormsDesigner
    Mar 9, 2011 at 18:29
  • 26
    I would have reduced the number of constants to just SECONDS_PER_DAY=86400. Why calculate something that won't change?
    – JohnFx
    Mar 9, 2011 at 18:41
  • 19
    What about leap seconds?
    – John
    Mar 9, 2011 at 23:41

10 Answers 10

Reset to default
47

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;
Improve this answer
9
  • 3
    +1 @kevin cline: I agree with your point about brevity a bug-hunting. The added benefit I see to using a named constant, especially when debugging, is that if it is discovered that a constant was defined incorrectly you only need to change one piece of code rather than searching through a whole project for all occurrences of the incorrectly implemented value.
    – oosterwal
    Mar 9, 2011 at 19:27
  • 47
    Or even worse: publid final int FOUR = 3;
    – gablin
    Mar 9, 2011 at 20:44
  • 3
    Oh dear, you must have worked with the same guy I once worked with.
    – quickly_now
    Mar 10, 2011 at 8:09
  • 2
    @gablin: To be fair, it is quite useful for pubs to have lids.
    – Alan Pearce
    Mar 10, 2011 at 9:24
  • 11
    I've seen this: public static int THREE = 3; ... note - no final!
    – Stephen C
    May 9, 2011 at 4:39
32

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.

Improve this answer
9
  • 13
    Getting into a crunch mode like you describe is a counterproductive antipattern that should be avoided. Programmers reach peak sustained productivity at about 35-40 hours/week.
    – btilly
    Mar 9, 2011 at 16:45
  • 4
    @btilly I wholeheartedly agree with you. But it happens, often due to external factors.
    – Vitor Py
    Mar 9, 2011 at 16:49
  • 3
    I general define constants for second, minute, day and hour. If nothing else '30 * MINUTE' is really easy to read and I know its a time without having to think about it.
    – Zachary K
    Mar 9, 2011 at 17:09
  • 9
    @btilly: Peak is 35-40 hours or a blood alchohol level between 0.129% and 0.138%. I read it on XKCD, so it's got to be true!
    – oosterwal
    Mar 9, 2011 at 18:46
  • 1
    @Vitor - Consider seconds_from_days() rather than CONVERT_DAYS_TO_SECONDS as seconds = seconds_from_days(num_days) gives better in-line readability where similar units are kept together.
    – Mark Booth
    Apr 16, 2011 at 23:11
9

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

Improve this answer
2
  • oh noes, so hours per day varies according to whether you refer to working hours per day (I work 7.5 BTW) or hours in a day. To change the meaning of the constant like that, you'd want to replace the name of it to something else. Your point about searching made easy is valid though.
    – gbjbaanb
    Feb 28, 2012 at 21:05
  • 2
    It appears that in this case HOURS_PER_DAY was not really the desired constant either. But being able to search for it by name is a huge benefit, even if (or especially if) you need to change it to something else in many places.
    – David K
    Jul 21, 2014 at 17:41
8

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.

Improve this answer
10
  • 2
    It's usually OK to use 0 or 1 I reckon. if(someList.Count != 0) ... is better than if(someList.Count != MinListCount) .... Not always, but generally.
    – Nobody
    Mar 9, 2011 at 16:36
  • 2
    @Dima: VS forms designer handles all that. If it wants to create constants, that's fine with me. But I'm not going into the generated code and replacing all hard-coded values with constants.
    – FrustratedWithFormsDesigner
    Mar 9, 2011 at 16:39
  • 5
    Let's not confuse code meant to be generated and handled by a tool with code written for human consumption though.
    – biziclop
    Mar 9, 2011 at 18:45
  • 2
    @FrustratedWithFormsDesigner as @biziclop pointed out, generated code is an entirely different animal. Named constants absolutely must be used in code that is read and modified by people. Generated code, at least in the ideal case, should not be modified at all.
    – Dima
    Mar 9, 2011 at 19:09
  • 2
    @FrustratedWithFormsDesigner: What happens when you have a well-known value hard-coded in dozens of files in your program that suddenly needs to be changed? For instance, you hard-code the value representing the number of clock-ticks per microsecond for an embedded processor then are told to port your software to a design where there are a different number of clock-ticks per microsecond. If your value had been something common, like 8, performing a find/replace on dozens of files could end up causing more problems.
    – oosterwal
    Mar 9, 2011 at 19:11
8

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.

Improve this answer
7

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
Improve this answer
4

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.

Improve this answer
5
  • 2
    Would you find it acceptable to hard-code the unchanging value 86400 instead of using the named constant SECONDS_PER_DAY? How would you verify that all occurrences of the value were correct and not missing a 0 or swapping the 6 ad 4, for instance?
    – oosterwal
    Mar 9, 2011 at 19:18
  • Then why not: seconds = num_days * 86400 ? That won't change either.
    – JeffO
    Mar 10, 2011 at 2:56
  • 2
    I've done the SECONDS_PER_DAY thing both ways - using the name and using the number. When you come back to the code 2 years later, the named number ALWAYS makes more sense.
    – quickly_now
    Mar 10, 2011 at 8:12
  • 2
    seconds = num_days * 86400 isn't clear to me. That's what ultimately counts. If I see "seconds = num_days * 24 * 60 * 60", aside from the fact that the variable names lend the meaning fairly well in this instance, I'd immediately ask myself why I separated them and the meaning becomes obvious because I left them as numbers (hence they're constant), not variables to which further investigation would require me to understand their values and if they are constant.
    – Neil
    Mar 10, 2011 at 16:55
  • 1
    What people often don't realise: If you change that lineOffset value from 24 to 25, you'll have to go through all of your code to see where 24 was used and if it needs changing, and then all the days to hours calculations multiplying by 24 really get into your way.
    – gnasher729
    Jan 24, 2017 at 18:23
3 
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.

Improve this answer
5
  • 2
    Would seconds = num_days * 86400 still be acceptable? If a value like that were used multiple times in many different files, how would you verify that someone had not accidentally typed seconds = num_days * 84600 in one or two places?
    – oosterwal
    Mar 9, 2011 at 19:22
  • 1
    Writing 86400 is very different from writing 24 * 60 * 60.
    – Carra
    Mar 9, 2011 at 21:59
  • 4
    Of course it will change. Not every day has 86,400 seconds in it. Consider, for example, daylight savings time. Once a year, some places only have 23 hours in a day, and another day they'll have 25. poof your numbers are broken.
    – Dave DeLong
    Mar 10, 2011 at 5:56
  • 1
    @Dave, excellent point. Leap seconds exist - en.wikipedia.org/wiki/Leap_second
    – user1249
    Mar 10, 2011 at 6:18
  • Fair point. Adding a function would be a safeguard if you ever need to catch those exceptions.
    – Carra
    Mar 10, 2011 at 9:42
3

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.

Improve this answer
1

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.

Improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.