Morbid use of constants

Question

Why should I write (as my collegue says):

import static org.apache.commons.lang.math.NumberUtils.INTEGER_ONE;
if (myIntVariable == INTEGER_ONE) { ... }

instead of:

if (myIntVariable == 1) { ... }

?

I know that the use of constants is recommended but I think the value of NumberUtils.INTEGER_ONE will never change! So I write 1.

Answer 1

You should not. The INTEGER_ONE name is no more meaningful than 1. If however this value has some other meaning (for example, month in the year), then using a constant (like Calendar.FEBRUARY) will make your code clearer.

I can guess that this constant in Commons Math library was created in Java 1.4 when there were no Integer cache and autoboxing, so it had sense in terms that you may reuse the same Integer object (not primitive int) in different places to save memory. So it was added for performance reasons, not for code clarity. Now it's obsolete: even if you need an Integer object, you can use Integer.valueOf(1) or implicit autoboxing and get the cached one.

Answer 2

You should not write INTEGER_ONE! Neither should you write 1 (see exception below)!

Why? A literal like 1 is called a magic number. Magic numbers are "unique values with unexplained meaning or multiple occurrences which could (preferably) be replaced with named constants" (explanation from the same Wikipedia page).

So what usually should be done is making those magic numbers to constants whose name represents or explains the meaning of that number. The constant INTEGER_ONE does not explain the meaning.

So what you actually have to do is to find the meaning of the value in this context and create a constant with exactly that name. If the 1 represents the maximum number of allowed threads for example, you should have a constant like:

static final int MAX_NUMBER_OF_THREADS = 1;

---

EDIT as per Tagir's comment

If the literal itself has a meaning in the domain for which you are writing the code, then it should not be replaced by a named constant. Tagir's example for calculating the inverse element is a good one:

double invert(double x) {
    return 1/x;
}

Here the literal 1 has a meaning in this context inside the math domain. So it can be used as is.

Answer 3

I happen to have just written style guidelines for my company and I'd suggest the following:

Don't use hard coded, "magic" values. If a value is constant, define it as such. Numbers such as -1, 0, 1, 2, 100 can be used in some situations.

My examples are in Objective-C as that's the language I was writing guidelines for, but the rules still apply.

Good Usage

static NSString* const DatabaseName = @"database.name";

//Acceptable use of "2"
float x = (ScreenWidth / 2) - (ImageWidth / 2);

//Acceptable use of 0
for (int i = 0; i < NumberOfItems; i++)

//Acceptable use of 100, but only because the variable is called "percentage"
float percentage = (someObjects * 100) / allObjects.count;

Bad Usage

float x = (480 / 2) - (120 / 2); //We have to guess these are sizes?

//Unneccessary constants.
for (int i = ZERO; i < NumberOfItems; i += ONE)

float percentage = (someObjects.count * 100) / 120; //What is 120?