Java Language Java Pitfalls - Nulls and NullPointerException Pitfall - "Making good" unexpected nulls
Example
On StackOverflow, we often see code like this in Answers:
public String joinStrings(String a, String b) {
if (a == null) {
a = "";
}
if (b == null) {
b = "";
}
return a + ": " + b;
}
Often, this is accompanied with an assertion that is "best practice" to test for null like this to avoid NullPointerException.
Is it best practice? In short: No.
There are some underlying assumptions that need to be questioned before we can say if it is a good idea to do this in our joinStrings:
What does it mean for "a" or "b" to be null?
A String value can be zero or more characters, so we already have a way of representing an empty string. Does null mean something different to ""? If no, then it is problematic to have two ways to represent an empty string.
Did the null come from an uninitialized variable?
A null can come from an uninitialized field, or an uninitialized array element. The value could be uninitialized by design, or by accident. If it was by accident then this is a bug.
Does the null represent a "don't know" or "missing value"?
Sometimes a null can have a genuine meaning; e.g. that the real value of a variable is unknown or unavailable or "optional". In Java 8, the Optional class provides a better way of expressing that.
If this is a bug (or a design error) should we "make good"?
One interpretation of the code is that we are "making good" an unexpected null by using an empty string in its place. Is the correct strategy? Would it be better to let the NullPointerException happen, and then catch the exception further up the stack and log it as a bug?
The problem with "making good" is that it is liable to either hide the problem, or make it harder to diagnose.
Is this efficient / good for code quality?
If the "make good" approach is used consistently, your code is going to contain a lot of "defensive" null tests. This is going to make it longer and harder to read. Furthermore, all of this testing and "making good" is liable to impact on the performance of your application.
In summary
If null is a meaningful value, then testing for the null case is the correct approach. The corollary is that if a null value is meaningful, then this should be clearly documented in the javadocs of any methods that accept the null value or return it.
Otherwise, it is a better idea to treat an unexpected null as a programming error, and let the NullPointerException happen so that the developer gets to know there is a problem in the code.
