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?

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.

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.

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.

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.

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.

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.

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.

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")));