A lot of example code posted on StackOverflow includes snippets like this:
if ("A".equals(someString)) {
// do something
}
This does "prevent" or "avoid" a possible NullPointerException
in the case that someString
is null
. Furthermore, it is arguable that
"A".equals(someString)
is better than:
someString != null && someString.equals("A")
(It is more concise, and in some circumstances it might be more efficient. However, as we argue below, conciseness could be a negative.)
However, the real pitfall is using the Yoda test to avoid NullPointerExceptions
as a matter of habit.
When you write "A".equals(someString)
you are actually "making good" the case where someString
happens to be null
. But as another example (Pitfall - "Making good" unexpected nulls ) explains, "making good" null
values can be harmful for a variety of reasons.
This means that Yoda conditions are not "best practice"1. Unless the null
is expected, it is better to let the NullPointerException
happen so that you can get a unit test failure (or a bug report). That allows you to find and fix the bug that caused the unexpected / unwanted null
to appear.
Yoda conditions should only be used in cases where the null
is expected because the object you are testing has come from an API that is documented as returning a null
. And arguably, it could be better to use one of the less pretty ways expressing the test because that helps to highlight the null
test to someone who is reviewing your code.
1 - According to Wikipedia: "Best coding practices are a set of informal rules that the software development community has learned over time which can help improve the quality of software.". Using Yoda notation does not achieve this. In a lot of situations, it makes the code worse.