Morbid use of constants

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.

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.

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.

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?

org.apache.commons.lang.math.NumberUtils.INTEGER_ONE it gives you a final static Integer object rather than primitive int 1, and as it is final static it acts as a constant and can be used in comparison of Integer objects because will always return same instance.

So in the above scenario it might not look fit but somewhere if you are using it while comparison, it for sure has impact.

Moreover, as much as we can, should prefer the use of constants over hardcoded beacuse:

1. It can make your code easily maintainable. If any situation occurs in future for change, you can change only at a single place.
2. The code looks cleaner & more readable.

You may know whether it will never change, but I won't if I start editing your code...

Basically it's a way of documenting your code in your actual code. The reason to use constants and examples like this is to avoid Magic Numbers in code and their disadvantages.

This being said, you can use it to a point where it is not advantageous anymore and clutter inducing. I tend to do it for things that are used more than once or have the concept of being changed by me or someone else... or in simpler terms important values.

From Class NumberUtils you'll see it's defined as:

/** Reusable Integer constant for one. */
public static final Integer INTEGER_ONE = new Integer(1)

So, you'll see that INTEGER_ONE is not the same as 1. It's an object that's already been constructed for you. So, if we're needing an instance of Integer(1), rather than create your own, you can reuse the one from the library saving time and memory.

It really depends on your application, if you indeed what the int version of 1, then, you'd probably be better of using that instead of this Integer class.

Imagine that you have this

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

But a few thousand times...

And suddenly in needs to be a 2.

What it easier for you to change?

EDIT: Before downvoting, im answering from the perspective of the advantages of not using magic numbers, im not in any way (i thought this was inferable, come on people) advising to change the library constant.