Java : "xx".equals(variable) better than variable.equals("xx") , TRUE?

Question

I'm reviewing a manual of best practices and recommendation coding java I think is doubtful.

Recomendation:

String variable;

"xx".equals(variable) // OK

variable.equals("xx") //Not recomended

Because prevents appearance of NullPointerException that are not controlled

Is this true?

Isidore

If you are writing a technical manual you have bigger grammar problems to worry about than a minor coding caveat like this. – Markusmarl

@Zak, that is not really relevant. How do you know the manual is going to be in English? Perhaps Xerg writes fluently in some other language, and is creating the manual in that language. Everyone who answered managed to figure out what the crux of the question was. – Straightout

I'm not writing any manual. I'm just checking. Excuse my English, I am a better reader than writer. – Isidore

Answer 1

This is a very common technique that causes the test to return false if the variable is null instead of throwing a NullPointerException. But I guess I'll be different and say that I wouldn't regard this as a recommendation that you always should follow.

• I definitely think it is something that all Java programmers should be aware of as it is a common idiom.
• It's also a useful technique to make code more concise (you can handle the null and not null case at the same time).

But:

• It makes your code harder to read: "If blue is the sky..."
• If you have just checked that your argument is not null on the previous line then it is unnecessary.
• If you forgot to test for null and someone does come with a null argument that you weren't expecting it then a NullPointerException is not necessarily the worst possible outcome. Pretending everything is OK and carrying until it eventually fails later is not really a better alternative. Failing fast is good.

Personally I don't think usage of this technique should be required in all cases. I think it should be left to the programmer's judgement on a case-by-case basis. The important thing is to make sure you've handled the null case in an appropriate manner and how you do that depends on the situation. Checking correct handling of null values could be part of the testing / code review guidelines.

Promiscuous

"It makes your code harder to read: "If blue is the sky..." -- otherwise known as yoda conditions - en.wikipedia.org/wiki/Yoda_conditions – Influx

Answer 2

It is true. If variable is null in your example,

variable.equals("xx");

will throw a NPE because you can't call a method (equals) on a null object. But

"xx".equals(variable);

will just return false without error.

Motheaten

@Tom: well, maybe. I've run into situations in which it should be an error and plenty of other situations in which it shouldn't be. I actually agree with Mark Byers that it makes sense to evaluate it on a case-by-case basis. – Motheaten

Answer 3

Actually, I think that the original recommendation is true. If you use variable.equals("xx"), then you will get a NullPointerException if variable is null. Putting the constant string on the left hand side avoids this possibility.

It's up to you whether this defense is worth the pain of what many people consider an unnatural idiom.

Naturalistic

Peronsalized I find the "Yoda version" very awkward to read, but it does indeed avoid the null pointer exception. The alternative is ' s!=null && s.equals("xx") ', which is more to type and takes a few extra nanoseconds to do the extra compare. – Bail

Answer 4

This is a common technique used in Java (and C#) programs. The first form avoids the null pointer exception because the .equals() method is called on the constant string "xx", which is never null. A non-null string compared to a null is false.

If you know that variable will never be null (and your program is incorrect in some other way if it is ever null), then using variable.equals("xx") is fine.

Hexahedron

Answer 5

It's true that using any propertie of an object that way helps you to avoid the NPE.

But that's why we have Exceptions, to handle those kind of thing.

Maybe if you use "xx".equals(variable) you would never know if the value of variable is null or just isn't equal to "xx". IMO it's best to know that you are getting a null value in your variable, so you can reasign it, rather than just ignore it.

Roe

Answer 6

You are correct about the order of the check--if the variable is null, calling .equals on the string constant will prevent an NPE--but I'm not sure I consider this a good idea; Personally I call it "slop".

Slop is when you don't detect an abnormal condition but in fact create habits to personally avoid it's detection. Passing around a null as a string for an extended period of time will eventually lead to errors that may be obscure and hard to find.

Coding for slop is the opposite of "Fail fast fail hard".

Using a null as a string can occasionally make a great "Special" value, but the fact that you are trying to compare it to something indicates that your understanding of the system is incomplete (at best)--the sooner you find this fact out, the better.

On the other hand, making all variables final by default, using Generics and minimizing visibility of all objects/methods are habits that reduce slop.

Nightclub

Answer 7

If you need to check for null, I find this better readable than if (variable != null && variable.equals("xx")). It's more a matter of personal preference.

Vintage

Answer 8

As a side note, here is a design pattern where this code recommendation might not make any difference, since the String (i.e. Optional<String>) is never null because of the .isPresent() call from the design pattern:

Optional<String> gender = Optional.of("MALE");
if (gender.isPresent()) {
    System.out.println("Value available.");
} else {
    System.out.println("Value not available.");
}
gender.ifPresent(g -> System.out.println("Consumer: equals: " + g.equals("whatever")));

Maje