When to catch a NullPointerException

Example

public Document readFoo(Document document) {
    try {
        return (Document) document.get("foo");
    } catch (NullPointerException npe) {
        Document doc = new Document();
        doc.put("error", "Document not found");
        return doc;
    }
}

Let’s just unpack what’s going on in the above:

  1. We receive an input and try to get the foo property from it.
  2. We may be returning a null
  3. The input, if null will cause the line of code to fail
  4. So we catch the NullPointerException and then create a new Document with an error message in
  5. The error message relates to the context of the calling function – which must have failed to find a document

How could this be done better?

ANYTHING would be better 😉

We have two options:

  1. Handle null more gracefully inside the function – which has the downside of not being able to tell apart the no inner document versus no provided document
  2. Make the function require a non-null input and solve it outside – you don’t need to explicitly do anything to force non-null input here. Conventionally we shouldn’t expect to ask a function to process a null

Refactored

// caller
Document result = Optional.ofNullable(findTheDoc())
    .map(doc -> readFoo(doc))  // could even be a method reference
    .orElseGet(() -> createNotFoundDocument());

public Document readFoo(Document document) {
    return (Document) document.get("foo");
}

public Document createNotFoundDocument() {
    Document doc = new Document();
    doc.put("error", "Document not found");
    return doc;
}

However, this does have a disadvantage… it conflates the not found with the no-child.

// caller
Document result = Optional.ofNullable(findTheDoc())
    .map(doc -&gt; readFoo(doc)) // we're now of type Optional<Optional<Document&gt;&gt;
    .orElseGet(() -&gt; Optional.of(createNotFoundDocument())
    .orElse(null);

public Optional<Document&gt; readFoo(Document document) {
    return (Document) document.get("foo");
}

public Document createNotFoundDocument() {
    Document doc = new Document();
    doc.put("error", "Document not found");
    return doc;
}

The above is actually worse than the original.

This is all down to the rather odd error handling implied by the original. At this point, you now have enough information to either:

  • Refactor some more so that null is the right answer to the no child scenario
  • Refactor better error handling for the no child
  • Rethink the whole thing

Conclusion

Catching NullPointerException tends to mask something more important. The Optional type in Java lets us model missing values well.

After refactoring our code, we realised that the original appeared to have three outputs, and this proved hard to do neatly, suggesting a major rethink was needed all along.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s