Is it a bad practice to check null before returning from a getter?

Question

I wanna prevent getters from returning null values, like the examples bellow. Is this a bad practice?

Example 1

public Integer getMinutes() {
   if (minutes == null)
       minutes = 0;
   return minutes;
}

Example 2

public List getTasks() {
   if (tasks == null)
       tasks = new ArrayList();
   return tasks;
}

Answer 1

The short answer is: it depends.

More specifically, the question isn't possible to answer in a general way without seeing the entire class and understanding its design. Null checks are an implementation detail, which are appropriate in certain designs, but might be a smell in others.

In general, methods must live up to their contracts, nothing more. So from the point of view of the API design, this question is irrelevant. You design your API following a long list of existing best practices, and then design your implementation to fit. One of those design best practices is that you shouldn't special case "zero elements" into null when returning a Collection. That is, if your method can reasonably return an empty collection, it should return an empty collection, rather than a special value like null. That almost always simplifies client code and removes a whole class of latent NPEs. This doesn't mean you can't use null internally to flag "zero elements", but it implies that if you do, you should perform a transformation on return (as in your examples).

So if your getTasks() method specifies that it always returns a (possibly empty) list, using a null check as in your example is a reasonable implementation choice. In your specific examples it may not be the best choice, since the JDK offers methods like Collections.emptyList<T>(), which allow you to cheaply re-use a singleton empty list instance, so you aren't saving anything in memory, and CPU savings are questionable¹.

Furthermore, by using the "null means empty list" convention, you require that all internal methods either have to check for tasks == null, or else use the getTasks() method. In the specific case of collection objects, I usually prefer to use the Collections.empty* methods rather that null to represent empty lists². Similar reasoning applies to the Integer case since Integer.valueOf(0) returns a singleton object on all compliant JVMs.

Those specific case aside, there are certainly reasons to use null checks to represent a missing or default element, particular when no good "empty" element presents itself or where performance is important. You will find this pattern used somewhat frequently across well designed libraries such as the Java runtime and Guava. So you can't say it's a bad practice. It is a practice to be carefully evaluated.

---

¹Use of these Collection.empty* methods applies also to your example. In your null case, you may consider directly returning Collections.emptyList(), rather than creating a new ArrayList on-demand. Whether that is appropriate depends on the overall class design, e.g., whether that list is mutable.

²Whether Collections.emptyList() is faster than an explicit null check is actually a complex question. If empty lists are so uncommon that they usually don't occur in a given process, the emptyList() approach is likely faster, since its cost (wrt the getter) goes to zero as the frequency of empty lists goes to zero. On the other hand, when the empty list does occur, it may create a polymorphic call site for all methods that use the List: every such site may see both a special "empty list" implementation and ArrayList. This can slow down the code, although a lot depends on inlining decisions. So, as usual with performance considerations, the exact answer is complex and you need to profile if the details matter.

Answer 2

In general, Yes, it is bad practice.

For example 1, if the value of the object (an Integer, in this case) is genuinely null, an API user would expect null to be returned. Not 0. For example, perhaps a user of your API sees 0 as an action taking 0 minutes, whereas null may be construed to mean that something never happened.)

For example 2, an API user would not expect a getter to have such a side effect such as constructing a new list.

Unless you include these behaviors in the contract (javadoc) of your methods, users can't be expected to realize what's actually going on, and that can cause them to work with incorrect data. Instead, they should be anticipating that null can come from these methods, and checking for it on their own.

You can help them, however, by using annotations such as javax.annotation.Nullable and javax.annotation.Nonnull to hint to users (and their IDEs) that the return value can or cannot be null, respectively. This way, they can intelligently anticipate and react to the null-ability of your method results.

Caveat: as Robert said in his answer, there are cases where lazy loading makes sense. But it must be done carefully, and only in contexts where it makes sense. Joshua Bloch wrote a sub-chapter about Lazy Initialization in his book, "Effective Java, Second Edition", where he explains that Lazy Initialization should be performed "judiciously" and effectively calls it a pre-optimization.

Answer 3

What you are describing is a design pattern called Lazy Loading (https://en.wikipedia.org/wiki/Lazy_loading). It is a good practice if used in certain contexts, but isn't a cure-all or something to just use for the fun of it.