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.

Syndicalism

It would make sense to use in this way, if it was your own custom constant. – Hematology

Maybe you can ask your colleague that question ? He/she may have a particular reason to use it that we don't know. – Dismay

@FlorentBayle My colleague uses constants EVERYWHERE! He replaces every number or string with a constant! And his answer to my Why is "The documentation says to use constants!"; yes, he is too nitpicking. – Syndicalism

@Michele: did you ask which documentation says to use constant? – Karolekarolina

Related: programmers.stackexchange.com/q/56375/88986 – Anaxagoras

The official code conventions mention how to deal with constants. No need to close as "opinion based" when there is an official guideline by the languages designers on how to deal with constants. – Ganesa

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.

Cutcherry

But you shouldn't be using == for Integer objects. – Predestine

For what it's worth, if using Calendar.FEBRUARY is more expressive than 1, then INTEGER_ONE is as well for the same reason, it's making it clear that it's meant simply as an integer and not some sort of ordinal. Although, I can't think of a situation where it'd be worth differentiating. – Fuzee

@Captain Man: there should be no reason to make clear that a number is meant to be a number as in good code, all literal numbers are meant to be a number as all others are represented by named constants. – Karolekarolina

Ironically, these constants in old code cause the opposite. As such old code uses new Integer(1), it’s the only place, where boxing 1 uses an unshared object. – Karolekarolina

@Karolekarolina Does "such old code" refer to old code in JDK ? I'm curious to see use of new Integer(1) as I can't imagine any good reason to use it. – Treasatreason

@Jean-FrançoisSavard, it's about Commons Math library. – Cutcherry

@TagirValeev Thanks, for the link. I really don't see any good reason to use new Integer(1) except if you hate your code-review team... I'm curious why they did not use valueOf instead. – Treasatreason

@Jean-FrançoisSavard, that's because this library preserves Java 1.4-compatibility which had no valueOf method. – Cutcherry

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.

Hulky

That's all good but you have to also reason with yourself on the justification of holding it in it's own variable. You should not do this with all numbers, not in a realistic sense at least. – Sorghum

@Sorghum Oh, and this is wrong. You should do this with nearly all literals that have a meaning. But this is a discussion that probably gets close to being opinion-based. – Hulky

Like when my professor asks for 'lots of comments' and then gets upset when you ask him what his definition of a lot is... then some people get marked down for having too many. – Sorghum

What about the method like this: double invert(double x) {return 1/x;}? Should I avoid 1 here as well? How to name the constant then? – Cutcherry

@TagirValeev Here the literal itself has a meaning in the domain (math): This calculation is used to get the inverse element. This is an obvious exception to the more general rule. – Hulky

Well the whole this question seems obvious to me, nevertheless it was asked. So probably you should note in your answer that some obvious exceptions exist. "Neither should you write 1" seems too strict statement for me... – Cutcherry

@TagirValeev I did that directly after writing my comment. – Hulky

Actually, in many coding standards I know, 0 and 1 are exempted from the No-Magic_Number rule for good reason, because the often don't represent a number wich can change in the future, but e.g. the presence or absence of something (similar to a bool) – Conciseness

@Conciseness I would not express the rule based on the value itself, but on the question whether or not the value has an implicit meaning in the current domain. Of course, the values 0 and 1 often have such an implicit meaning. And even if a value can't change (which then is a clear domain rule), this does not mean to not give them an expressive name. In fact, this is all about code readability. – Hulky

@Seelenvirtuose: I agree, 0 and 1 are just the most common literals, that have a clear meaning on their own. Like in the example you provided or e.g. if (size > 0). I think no one would require this to be written as if (size > NONE). – Conciseness

0, 1, and occasionally 2 are generally considered acceptable. Magic numbers to be avoided are those which either a) are potentially subject to future change or b) have a meaning which is not immediately obvious to all. Only rarely will 0 or 1 have either of these properties. – Teryn

@Tagir Valeev: the correct name for the 1 constant in 1/x is IDENTITY_ELEMENT_WITH_RESPECT_TO_THE_DIVISION_BINARY_OPERATION. Don’t be surprised that I also prefer using a literal 1 there… – Karolekarolina

@Holger, I know a developer which uses even longer method and class names. – Cutcherry

@TagirValeev but dude! Tab completion!! who cares how long the name is? i still type it in ~3 key presses! /s – Pelite

@basher: I hate needing horizontal scrollbars when looking at source code… – Karolekarolina

@Karolekarolina Agreed. you missed my /s. – Pelite

I would suggest that literals are also appropriate in many cases where the structure of some code is strongly related to the value. For example, if a routine to sort some portion of a double[] will accept any size range, but will most often be called with ranges of 5 or fewer items, using a switch and using literal constants for the cases would be entirely reasonable. If a piece of code is going use a hard-coded sequence of comparisons to sort 5 items (such a sequence may be much faster than a loop), labeling that case with the literal 5 will be better than any other identifier. – Xenocryst

t @TagirValeev In an example where we are assigning a value from an array constan like for default value at subscript 0 from an array constant of 3 elements. we are using var defaultCountryIndia = countries[0]; instead of using 0 we use DEFAULT_COUNTRY_INDEX. So tomorrow even if the position in array changes we just need to modify the constant file. Is this correct from point of view of exceptions for magic number 0 and 1? – Kerchief

I know this is ancient history, but this answer has too many upvotes. I really don't agree with this. The accepted answer is better, but I'm going to write my own for the possible help of future developers. – Platelayer

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?

Orva

Answer 4

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.

Mellins

I agree with the first part of your answer, but the two general reasons you gave for using constants just don't apply here. – Conciseness

@Conciseness i provided them not for supporting my statement but as a normal coding convention. – Mellins

Then I'd recommend you reword that part a a bit. At thee moment you are saying: always prefer constants, in the context of a question where you should not prefer that constant. – Conciseness

@Conciseness edited, Thanks for pointing it out. Its always good to learn from you guys :) – Mellins

Re your first paragraph: Does Java consider keys of the same value, but from different origins, to be different when checking containers? If so, that sounds horrible. – Holter

@Holter if you are asking regarding this :- Integer i = new Integer(1); Integer k = new Integer(1); , then here i, k are different objects. – Mellins

@AnkitNigam They may be different objects, but it's not true at all that this "has impact" in HashMap or HashSet, because they'll compare the same using .equals. – Garrity

Answer 5

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.

Sorghum

I disagree with the magic number argument. INTEGER_ONE is always going be 1. – Lately

@CKing What does INTEGER_ONE mean? Is it how many times you can guess your login details before the world implodes or is it something simpler? – Sorghum

That is exactly the point: INTEGER_ONE had no more meaning than a litteral 1, so as far as code clarity and maintainability are concerned it offers no advantages. – Conciseness

Then the entire question would be irrelevant and his issue would be towards bad variable naming conventions than magic numbers. I was just giving him the benefit of the doubt and assuming his poor variable name was a 'slip of the tongue'. – Sorghum

@insidesin: please note that it’s not him who named that variable – Karolekarolina

Yeah @Karolekarolina read below for my silly 'doh' moment... well if it's not deleted for you that is, it is for me :| – Sorghum

Answer 6

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.

Vitrify

Doesn't Integer.valueOf(int) do essentially the same? – Farflung

Answer 7

The accepted answer is quite good, but I wanted to add my judgment to this because I think it's a bit different from that of others.

You should use a named constant in three situations:

1. When the value behind the constant has some cost to it, that you can avoid by comparing constants instead
2. When the value behind the constant is ambiguous in meaning, and giving it a meaningful name would help with comprehension
3. When the value behind the constant is used in multiple places, and you would logically want all of them to change at the same time.

For case #1, an example would be the use of something like new Integer(4967). The Integer class has static constants for a bunch of values, but probably not 4967. If you make it a constant, you save slightly on memory, significantly on boxing/unboxing costs, and you can use == to compare identity instead of having to use .equals() for a correct comparison. This makes your code shorter, easier to read, and more efficient.

For case #2, the accepted answer's example of months is good, if slightly weak. Most people know the order of the months off by heart and wouldn't be confused to read code that says month == 6; but perhaps people who use a different native calendar, or children perhaps, might not know it as well. Reading month == FEBRUARY is going to be equally easy for most people. It also protects you from accidentally writing month == 14 (though I might argue that in this case, an enumerated type would be even better, making an invalid month assignment a compile-time error instead).

In general I would use a named constant for anything where the name itself carries additional meaning beyond the number. Months, TCP ports, special values (PI or Integer.MAX_VALUE), and so on.

Case #3 is architectural, and it's as much about program correctness as readability. There is some nuance here as well. The idea is to understand the logical connection between values, and use constants to connect up things that logically go together. The nuance is that you want to do this only when there is a logical connection between the values--not just because they happen to have the same value.

For example (a little contrived, but I think it works okay), say you're writing an internet service that hosts in both TCP and UDP. You could set both protocols to use the same port number (say 6666) by default; but they are going to be contacted by separate services on the client. There is no reason they have to be the same; someone might have to change them to get around firewall restrictions.

So in this case, I would use two constants, TCP_PORT = 6666 and UDP_PORT = 6666. I wouldn't put the value directly in the code, unless it was only going to be used exactly once, and the code was simple enough that anyone would be able to find and configure it (an example might be builder-like code that looks more like a DSL than an actual program). But I also wouldn't fold the two constants together into a single PORT constant, because the two values just happen to be the same: they aren't bound to one another. Folding them together would just require someone to unfold them at some point down the road, which would require changes throughout the module.

TLDR: Use constants when they make your code more maintainable, more readable, and/or faster (roughly in that order). Don't use them if they make your code less readable or less maintainable. In many cases I'd accept a small reduction in performance, if it made the code significantly more readable.

Platelayer

Answer 8

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.

Pantisocracy

So you expect OP to change the Commons Math library and set INTEGER_ONE to 2 there? – Cutcherry

so you are suggesting to alter INTEGER_ONE == 2? – Entophyte

I think that the question is more oriented to the use of constants, not the apache specific INTEGER_ONE – Pantisocracy

@Luis: On the contrary,the second part shows that this question is explicitly about INTEGER_ONE – Conciseness

@Conciseness There is no proof that he meant to name his variable poorly or not and will not be any help until you ask him (hence no argument here, completely on the fence, neither of you are right or wrong) – Sorghum

@insidesin: He didn't name the constant it comes from the library. And that constant is NOT poorly named. It's purpose is to serve as a number of a certain type (different from the litteral 1). It's purpose was never to be a descriptive name for a certain parameter (which is what most people think of when they say "prefer constants"). So all arguments for not using 1 directly in the code because it is a magic number also apply directly to INTEGER_ONE, plus it is more boilerplate and thus less readable. Tagir Valeev explains, why it's also no longer necessary from a performance point of view. – Conciseness