2

I'm trying to avoid overdo null checks, but at the same time I want to make null check whenever it needs to make the code robust. But sometimes I feel like it starts to be so much defensive since I don't implement API. Then I avoid some null-checks but when I start unit tests, it starts to always wait for runtime exceptions. What is the right approach, how to feel the balance. Here are the notes I have collected so far:

  • avoid unnecessary null-checks in private methods of the instance class since proper null checks are made before and instance class is responsible of this, not each function
  • make null checks in public methods and explain function behavior in the Contract for the public usage
  • make null checks in the constructor

Let me give an dummy example where my confusing starts:

Here is the default function of the interface:

default void someFunction() {

// instantiaded foo and filters
foo = processFoo(foo, filters);

// do some operations with foo
// null check if processFoo's contract returns null (description in the implementation)

}

Implementation:

Foo processFoo(foo, filters) {
// Should I null check foo and filters?

// Operations with foo.someFields
// Operations with filters.someFields

return foo; // If I null check then I should return some exception or null to the caller function. 
// If I don't null check just return received foo.
}
cmlonder
  • 2,370
  • 23
  • 35
  • 1
    My rule of thumb is never to return a null from my methods, but an empty object. – zuckerburg Jan 03 '19 at 08:16
  • @zuckerburg Does that mean you return an object in invalid state instead of null? – Torben Jan 03 '19 at 08:18
  • @Torben It means to create something like `public static final Foo EMPTY = new Foo()` and let the method return `FOO`. – Jai Jan 03 '19 at 08:21
  • just an empty object, so if, for example, it's a list that I iterate over on the caller, it won't fail because it's null, it just has 0 rows. or if it's an object like user details, the caller will return an empty object, which means what you see is an empty user object, instead of null – zuckerburg Jan 03 '19 at 08:21
  • For better or for worse, the null object pattern isn't a standard Java idiom, although at least "empty collection" and some similar individual cases are. – chrylis -cautiouslyoptimistic- Jan 03 '19 at 08:22
  • 3
    To avoid Null checks and NPE Optional feature was introduced. Take a look at https://www.oracle.com/technetwork/articles/java/java8-optional-2175753.html – Sangam Belose Jan 03 '19 at 08:23
  • Which you are talking about like Optional class in Java. But I think i was talking about different scenario, it doesn't need to return new Foo, it may be a void function too, same null check question still exists – cmlonder Jan 03 '19 at 08:23
  • I think if you have control over method call which is processFoo(foo, filters) then you can make sure while calling that you don't pass null object which will make your life easier in other method and there you don't end up doing null check. – Eshu Jan 03 '19 at 08:23
  • possible duplicate of [What is a NullPointerException, and how do I fix it? ](https://stackoverflow.com/q/218384/4574765) – Jatin Asija Jan 03 '19 at 08:24
  • The question you should be making is, ¿does it make sense returning a null here? ¿what will the code do after it, will it work with that null or it will exit from the method? My rule is, if there's nothing to do with it, don't check it and just catch the exception: this way you could easily monitor the problem. If the program can and SHOULD continue with its job even with the object being null, THEN check for it. – aran Jan 03 '19 at 08:26
  • 1
    @zuckerburg It sounds to me that you've taken the legitimate empty-list-instead-of-null practise and extended it to where it doesn't belong. Returning an empoty and invalid object instead of null still requires you to make the check. If you forget that check (which will necessarily be more complicated than "== null") you pass the invalid object to a method that doesn't allow nulls and the error is potentinally sent way further down the call stack (and made more obscure). I'd strongly advise against empty objects (other than collections). – Torben Jan 03 '19 at 10:37
  • @Jai Comparing a return value to a constant is no way different than comparing to a null. What's worse, when passed on (e.g. due to an error) the constant looks no different than intentionally passed object, so it won't even be caught by possible NonNull annotations. Null is better than an invalid empty object as a method can be documented and annotated to not allow null values and then encountering a null immediately means an unintentional programming error. – Torben Jan 03 '19 at 10:41
  • @Torben I do know it's not a wonderful design, but more of what I think zuckerburg meant. Interestingly, JavaFX does use this design for some (e.g. [`Background.EMPTY`](https://openjfx.io/javadoc/11/javafx.graphics/javafx/scene/layout/Background.html#EMPTY)), but probably not related to the context of the question. – Jai Jan 04 '19 at 01:24

3 Answers3

4

On a general rule, I never check for nulls when I'm in control of the calls, but if my code can be called by someone else, I do enforce checks, since you're talking about an API spec, you may check input parameters and throw IllegalArgumentException.

Of course, this it's not required, most of the times, when you have an NPE, there's an underlying bug somewhere, however if you are not in total control of calls, you will definitely need some extra effort. (expecially if your API will be called frequently, you need something to help you trace/identify bad calling patterns).

When designing an API, aside the architecture itself, you have to focus also on maintenance/troubleshooting and security, expecially if the API is publicly exposed (which means also sanitizing each and every parameter).

BigMike
  • 6,683
  • 1
  • 23
  • 24
1

If you have implemented layering in your project, good place to do null checks are the layers that receives data externally. Like for example: the controllers, because it receives data from the user... or the gateways because it receives data from repositories.

But if you have the capabilities to control the return values of you method to not return null, that would be good. This will enable you to write code with less null checks, because you can just "ostrich" your way thru such missing values. Java provided us with such classes/methods such as Optional, Java 11's Reader.nullReader(), Writer.nullWriter(), and empty collections from Collections.empty{List|Map|Set|...}().

Of course, you can make your own null objects so that it can polymorph in a way that it won't affect the program when it is used.

sweet suman
  • 1,031
  • 8
  • 18
1

Going to play devil's advocate here because it seems like some individuals commenting seem to dislike the concept of 'null' or using it in their programs.

'null' is not something to avoid in the slightest, and if you try to, you'll be met with defeat; creating objects and allocating space in memory for such objects is unnecessary, because if your code would've broke when receiving a 'null' value, it would definitely also break when dealing with an object that it simply can't handle. Not only this, but you'll similarly be met with defeat upon using many of Java's methods.

If you can avoid using 'null' and give your code something to work with in a better way, do so. Otherwise, don't break your back and spine in order to avoid it.

Finally, to answer your question:

You input a null-check when you're not sure if something, like a method, could return a 'null' value. For instance, if you call a method that you did not create, and the method is designed to return an object, and it may return a null value, add in a special check for that case. If you know that you'll never receive a null value, like if you designed a method yourself such that it never returned 'null', you don't need to put in any null checks.

Moreover, if you are using only internal or private methods, and you know you're not passing in null parameters, like in your example, it is also unnecessary to add in null checks. This also applies to the constructor instance that you mentioned. Otherwise, as a rule of thumb in public methods, it is usually good to use null-checks, even if only to throw an IllegalArgumentException or to return failure.

Sal
  • 302
  • 1
  • 11
  • When you're not sure if something _could_ return null, then the API is broken and must be fixed so you're sure it can return null. Then... when you're sure it can return null you document it by returning an Optional instead of a direct, nullable, object reference. – Torben Jan 03 '19 at 10:48
  • 1
    I was not talking about in the case of not knowing what an API method could return, but rather, in the case of having possible values passed to a method that will return null. Consider having a List containing 'null' at index 4, and you call list.get(4), returning a null value for data you might've expected to be non-null. In this case, you would require a null-check because you're not sure if the method will return null. It doesn't imply a broken method; it implies an unexpected error that should be accounted for. Using an Optional, or anything similar, would be effectively the same. – Sal Jan 03 '19 at 10:55