Stream of Null

code-review-image-1024x5202x

I quite like refactoring code from Java 7 (and lower) syntax into the Java 8 streams. I find the streams to be more expressive of the intent of the code and the boilerplate of the long-winded version seems evaporate, leaving something neater in its place. For example:

for(MyPojo pojo:listOfMyPojos) {
   // find and return the first one with a long title
   if (pojo.getTitle().length() > 12) {
       return pojo.getBody();
   }
}

// not found
return null;

The above code is not too bad, but is essentially finding the first thing in a list, and would be nicer if we wrote it as a stream:

return listOfMyPojos.stream()
   .filter(pojo -> pojo.getTitle().length() > 12)
   .map(MyPojo::getBody)
   .findFirst()
   .orElse(null);

Cool. Less code. What could possibly go wrong.

Uh Oh!

Here’s what:

// try the above code with the following in the list
MyPojo naughtyPojo = new MyPojo(
    "This is a very long title but the object has no body",
    null);

The map function within the stream will, if that object is the first found, map the MyPojo to null. So what? It makes findFirst explode!

In other words there’s a hidden use-case in the original implementation, which is that it’s possible to return null both when nothing is found, or when the thing found is null.

The Rash Fix

With Streams, findFirst is expecting to find a non-null. The following code might make your null pointer exceptions go away with the Streaming implementation:

return listOfMyPojos.stream()
   .filter(pojo -> pojo.getTitle().length() > 12)
   .map(MyPojo::getBody)
   .filter(Objects::nonNull)
   .findFirst()
   .orElse(null);

But the above has different behaviour in that it no longer finds the body of the first object with a long title – it finds the first object with both a long title and a non-null body… which is not what the original code was intending (it might be better… but these sorts of edge cases, unless heavily tested, are where bugs lie).

What To Do?

Luckily there’s a Java 8 solution to the rescue. Optional’s map function is the answer. You can perform map on optional once you’ve found what’s provably the first item you want to get, to convert it into the form you want to return. So the original method above is correctly refactored thus:

return listOfMyPojos.stream()
   .filter(pojo -> pojo.getTitle().length() > 12)
   .findFirst()
   .map(MyPojo::getBody)   // this is Optional.map, not Stream.map
   .orElse(null);
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