One of the first things I like to establish in a new project is the use of tools like Checkstyle and Findbugs in order to codify some code guidelines and to avoid bugs that can be determined by static code analysis.

Sooner or later with these tools one stumbles over a case where people have the feeling it is going to far. One such case is the check for Magic Numbers of Checkstyle. It will warn about any use of numeric literals except -1, 0, 1 and 2 that aren't used to define a constant.

Many developers have a problem with this check as one can see from the resulting code. I have seen code like this

private static final int FOUR = 4;

and also

private static final int FOUR = 5;

and my all time favorite (I'm not making this up!):

firstname = rs.getString(1);

lastname = rs.getString(2);

city = rs.getString(2 + 1);

zip = rs.getString(2 + 2);

country = rs.getString(2 + 2 + 1);

But there is a different case where the discussion comes up. It's with obvious constants like 100 for converting fractions into percent values or 1024 for conversion between bytes and kibibytes. Some argue these aren't magic numbers (or magic numbers but not as bad) because their meaning is obvious and they won't change.

I disagree and will vote to make them constants any time. Here is why:

1. The meaning is NOT obvious. What does value * 100 mean? Is a fraction converted to percent? Is a length measured in meters, converted to centimeters? Or something multiplied by a rough approximation of g*g with g being the gravitational acceleration on earth? Or maybe I'm multiplying something with the size of an array, that happens to be 100. Can't tell. A simple constant with a proper name will fix that.

2. Ok, I do agree, many of these constants won't change. But the purpose of defining constants (or methods, or classes) is not (only) to enable later change, but to make reading, understanding and reasoning more easy. So the question if a value might change in the future is completely irrelevant.

3. (This one is the argument that I'm missing in most of the discussions) I just don't want to think about it or have others think about it. I have seen dozens, probably hundreds of instances, where a properly named constant would have helped tremendously understanding a piece of code. And I have seen very few cases where it might have hurt readability and not a single one where it hurt badly. Therefore: just extract a constant and be done with it.

Note: Just because it is a constant doesn't mean it has to be public, or even a class level field. If it is used only in a single method a local variable is just fine.