To avoid magic numbers, we often hear that we should give a meaningful name for a variable. Such as:

//THIS CODE COMES FROM THE CLEAN CODE BOOK
for (int j = 0; j < 34; j++) {
    s += (t[j] * 4) / 5;
}

-------------------- Change to --------------------

int realDaysPerIdealDay = 4;
const int WORK_DAYS_PER_WEEK = 5;
int sum = 0;
for (int j = 0; j < NUMBER_OF_TASKS; j++) {
    int realTaskDays = taskEstimate[j] * realDaysPerIdealDay;
    int realTaskWeeks = (realdays / WORK_DAYS_PER_WEEK);
    sum += realTaskWeeks;
}

I have a dummy method like this:

Explain: I suppose that I have a list people to serve and by default, we spend $5 to buy food only, but when we have more than one person, we need to buy water and food, we must spend more money, maybe $6. I'll change my code, please focus on variable 1, my question about it.

public int getMoneyByPersons(){
    if(persons.size() == 1){ 
        // TODO - return money for one person
    } else {
        // TODO - calculate and return money for people.
    }

}

When I asked my friends to review my code, one said giving a name for the value 1 would yield cleaner code, and the other said we don't need a constant name here because the value is meaningful by itself.

So, my question is Should I give a name for the value 1? When is a value a magic number and when is it not? How can I distinguish context to choose the best solution?

  • Where does persons come from and what does it describe? Your code has no comments whatsoever so its hard to guess what it's doing. – Hubert Grzeskowiak Sep 11 at 3:21
  • 5
    It does not get any clearer than 1. I would change "size" to "count" though. And moneyService is stupid as it is, it should be able to decide how much money to return depending on the persons collection. So I would pass persons to getMoney and let it sort out any exceptional cases. – Martin Maat Sep 11 at 5:26
  • 3
    You could also extract the logical expression from your if clause to a new method. Maybe call it IsSinglePerson(). That way you extract a little more than just that one variable but also make the if clause a little more readable... – selmaohneh Sep 11 at 5:41
  • 3
    Possible duplicate of Is every number in the code considered a "magic number"? – gnat Sep 11 at 6:28
  • 2
    Perhaps this logic would be better suited in moneyService.getMoney()? Would there ever be a moment where you'd need to call getMoney in the case of 1 person? But I would agree with the general sentiment that 1 is clear. Magic numbers are numbers where you'd have to scratch your head asking how the programmer arrived at that number.. i.e. if(getErrorCode().equals(4095)) ... – Neil Sep 11 at 6:38
up vote 17 down vote accepted

No. In that example, 1 is perfectly meaningful.

However, what if persons.size() is zero? Seems strange that persons.getMoney() works for 0 and 2 but not for 1.

  • I agree that 1 is meaningful. Thanks, by the way, I updated my question. You can see it again to get my idea. – Jacky Sep 12 at 7:01

Why does a piece of code contain that particular literal value?

  • Does this value have a special meaning in the problem domain?
  • Or is this value just an implementation detail, where that value is a direct consequence of the surrounding code?

If the literal value has a meaning that is not clear from context, then yes, giving that value a name through a constant or variable is helpful. Later, when the original context is forgotten, code with meaningful variable names will be more maintainable. Remember, the audience for your code is not primarily the compiler (the compiler will happily work with horrible code), but future maintainers of that code – who will appreciate if the code is somewhat self-explaining.

  • In your first example, the meaning of literals such as 34, 4, 5 is not apparent from the context. Instead, some of these values have a special meaning in your problem domain. It was therefore good to give them names.

  • In your second example, the meaning of the literal 1 is very clear from the context. Introducing a name is not helpful.

In fact, introducing names for obvious values can also be bad as it hides the actual value.

  • This can obscure bugs if the named value is changed or was incorrect, especially if the same variable is reused in unrelated pieces of code.

  • A piece of code might also work fine for a specific value, but might be incorrect in the general case. By introducing unnecessary abstraction, the code is no longer obviously correct.

There is no size limit on “obvious” literals because this depends totally on context. E.g. the literal 1024 might be totally obvious in the context of file size calculation, or the literal 31 in the context of a hash function, or the literal padding: 0.5em in the context of a CSS stylesheet.

There are several issues with this piece of code, which, by the way, can be shortened like this:

public List<Money> getMoneyByPersons() {
    return persons.size() == 1 ?
        moneyService.getMoneyIfHasOnePerson() :
        moneyService.getMoney(); 
}
  1. It is unclear why one person is a special case. I suppose that there is a specific business rule which tells that getting money from one person is radically different from getting money from several persons. However, I have to go and look inside both getMoneyIfHasOnePerson and getMoney, hoping to understand why are there distinct cases.

  2. The name getMoneyIfHasOnePerson doesn't look right. From the name, I would expect the method to check if there is a single person and, if this is the case, get money from him; otherwise, do nothing. From your code, this is not what is happening (or you're doing the condition twice).

  3. Is there any reason to return a List<Money> rather than a collection?

Back to your question, because it is unclear why there is a special treatment for one person, the digit one should be replaced by a constant, unless there is another way to make the rules explicit. Here, one is not very different from any other magic number. You could have business rules telling that the special treatment applies to one, two or three persons, or only to more than twelve persons.

How can I distinguish context to choose the best solution?

You do whatever makes your code more explicit.

Example 1

Imagine the following piece of code:

if (sequence.size() == 0) {
    return null;
}

return this.processSequence(sequence);

Is zero here a magical value? The code is rather clear: if there are no elements in the sequence, let's not process it and return a special value. But this code can also be rewritten like this:

if (sequence.isEmpty()) {
    return null;
}

return this.processSequence(sequence);

Here, no more constant, and the code is even clearer.

Example 2

Take another piece of code:

const result = Math.round(input * 1000) / 1000;

It doesn't take too much time to understand what it does in languages such as JavaScript which don't have round(value, precision) overload.

Now, if you want to introduce a constant, how would it be called? The closest term you can get is Precision. So:

const precision = 1000;
const result = Math.round(input * precision) / precision;

Does it improve readability? It might be. Here, the value of a constant is rather limited, and you may ask yourself if you really need to perform the refactoring. The nice thing here is that now, the precision is declared only once, so if it changes, you don't risk making a mistake such as:

const result = Math.round(input * 100) / 1000;

changing the value in one location, and forgetting to do it in the other one.

Example 3

From those examples, you may have an impression that numbers should be replaced by constants in every case. This is not true. In some situations, having a constant doesn't lead to code improvement.

Take the following piece of code:

class Point
{
    ...
    public void Reset()
    {
        x, y = (0, 0);
    }
}

If you try to replace zeroes by a variable, the difficulty would be to find a meaningful name. How would you name it? ZeroPosition? Base? Default? Introducing a constant here would not enhance the code in any way. It would make it slightly longer, and just that.

Such cases are however rare. So any time you find a number in code, make the effort trying to find how the code can be refactored. Ask yourself if there is a business meaning to the number. If yes, a constant is mandatory. If no, how would you name the number? If you find a meaningful name, that's great. If not, chances are you found a case where the constant is unnecessary.

  • I would add 3a: Don't use the word money in variable names, use amount, balance or the like. A collection of money does not make sense, collections of amounts or balances does. (OK I do have a small collection of foreign money (or rather coins) but that is a different non-programming matter). – Bent Sep 11 at 21:22

You could make a function that takes a single parameter and returns times four divided by five that offers a clean alias into what it does while still using the first example.

I'm just offering my own usual strategy but maybe I'll learn something too.

What I am thinking is.

// Just an example name  
function normalize_task_value(task) {  
    return (task * 4) / 5;  
}  

// Is it possible to just use tasks.length or something like that?  
// this NUMBER_OF_TASKS is the only one thats actually tricky to   
// understand right now how it plays its role.  

function normalize_all_task_values(tasks, accumulator) {  
    for (int i = 0; i < NUMBER_OF_TASKS; i++) {  
        accumulator += normalize_task_value(tasks[i]);  
}  

Sorry if I'm off base but I'm just a javascript developer.

  • 4
    But what do those values 4 and 5 mean? If one of them ever needs to change, would you be able to find all places where the number is used in a similar context and update those locations accordingly? That is the crux in the original question. – Bart van Ingen Schenau Sep 11 at 7:33
  • I think the first example was pretty dang self explanatory so that might not be one I can answer. The operation should not change in the future, when you need to I'd write a new one to accomplish whats needed. Comments are what I think would most benefit this purpose. Its a one line call to reduce in my language. How important is it to make absolutely understandable to the layperson? – etisdew Sep 11 at 10:14
  • @BartvanIngenSchenau The whole function is improperly named. I would prefer a better function name, a comment with a spec what the function is supposed to return, and a comment why * 4 / 5 achieves that. Constant names are not really needed. – gnasher729 Sep 11 at 23:21
  • @gnasher729: If the constants appear exactly once in the source code of a well named, well documented function, then it might not be needed to use a named constant. As soon as a constant appear multiple times with the same meaning, giving that constant a name ensures that you don't have to figure out if all those instances of the literal 42 mean the same thing or not. – Bart van Ingen Schenau Sep 12 at 7:59
  • At the heart of it, if you expect you may need to change the value yes. I would change it to a const to represent the variable, however. I don't think it is the case and this should simply be an intermediate variable residing in the scope above it. I don't want to cop out and say make everything a parameter to a function but if it can change but rarely I would make it an internal parameter to that function or object/struct scope where you're currently working to make that change leaving the global scope alone. – etisdew Sep 12 at 10:16

That number 1, could it be a different number? Could it be 2, or 3, or are there logical reasons why it must be 1? If it must be 1, then using 1 is fine. Otherwise, you can define a constant. Don't call that constant ONE. (I have seen that done).

60 seconds in a minute - do you need a constant? Well, it is 60 seconds, not 50 or 70. And everyone knows it. So that can stay a number.

60 items printed per page - that number could have easily been 59 or 55 or 70. Actually, if you change the font size, it might become 55 or 70. So here a meaningful constant is more asked for.

It's also a matter how clear the meaning is. If you write "minutes = seconds / 60", that's clear. If you write "x = y / 60", that's not clear. There must be some meaningful names somewhere.

There is one absolute rule: There are no absolute rules. With practice you will figure out when to use numbers, and when to used named constants. Don't do it because a book says so - until you understand why it says that.

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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