How to Make Your Reviewer Cry Using Java Optional

I think code review is one of the best sources of inspiration. I see it as an opportunity to learn new things from other software developers sending pull/merge requests. Moreover, sometimes you may need to study a specific subject with more details regarding the under review code. Usually, this process leads to a more profound knowledge of that domain. However, there's still another fact about code review: after a while, you face some common mistakes. Recently, I reviewed a feature in which I saw an Optional pitfall. Honestly, I had seen that issue several times in different merge requests sent by several developers (from beginners to experienced ones).

I know that there are tonnes of articles regarding Java Optional some of which are quite useful like this one (although I have arguments against some of the cases mentioned in the article) by Anghel Leonard, the author of Java Coding Problems. I don't want to repeat all of the contents you can simply find by googling. I intend to talk about some of the most common and less addressed pitfalls regarding Optional usage.

What is Optional?

Before diving into the main subject, lets review some definitions and see what Optional is. As a Java developer, you should have encountered the infamous NullPointerException that occurs when you're going to access a null reference. With a quick search, you can find thousands of memes and jokes about null references in Java (and other languages as well). Optional came in to play in Java 8 to help programmers get rid of all of the problems caused by null references. Have a look at this article, from Oracle technical resources, to read more about the motivation of Optional existence in Java.

Now we are going to have a look at Optionals JavaDoc and check the provided definition.

A container object which may or may not contain a non-null value.

Following this introduction, as an API note, there is an answer to the first question that may come to mind: when should we use Optional?

Optional is primarily intended for use as a method return type where there is a clear need to represent "no result," and where using null is likely to cause errors. A variable whose type is Optional should never itself be null; it should always point to an Optional instance.

So we can extract the following points from the JavaDoc descriptions:

• Optional is a container object.
• It may not contain a non-null value.
• It is mainly used as a return type when using null is troublesome.
• The variable with the Optional type should not be null.

Keeping in mind these subtle points, you can simply spot most of the Optional related code smells. Now, its time to go through the four most common Optional pitfalls.

Old Fashioned Exception Handling

One of the most common mistakes I usually face in code reviews is using an if/else block to handle the circumstances that the Optional variable doesn't contain a non-null value (as previously mentioned in the Optional
definition). Check this out:

Optional<User> userOptional = userDao.findByCellNumber(cellNumber);if (!userOptional.isPresent()) {    throw new UserNotFoundException("...");} else {    User user = userOptional.get();    //...}

Suppose that you intend to find a user by a specific cell number from the database. Then you receive an Optional variable from the corresponding DAO class. Now, you want to check if there's a user with the cell number you have provided. What's wrong with the above code? Doesn't work? Honestly, it works like a charm. However, it's not elegant; it's something like twisting wood screws with a wrench. Now, check the following code snippet:

User user = userDao.findByCellNumber(cellNumber).orElseThrow(UserNotFoundException::new);

Using the orElseThrow, you can do the same thing more oncisely and elegantly without any need to get method.
You can also use its no-argument version of orElseThrow introduced in Java 10.

orElse and orElseGet Misuse

The second common pitfall may emerge while using orElse or orElseGet. It seems these methods do the same thing; however, there is a significant difference not that fancy like the previous one. Lets have a look at their signatures and corresponding Javadoc:

public T orElse(T other)//If a value is present, returns the value, otherwise returns other.//Parameters://other - the value to be returned, if no value is present. May be null.//Returns://the value, if present, otherwise other

public T orElseGet(Supplier<? extends T> supplier)//If a value is present, returns the value, otherwise returns the result produced by the supplying function.//Parameters://supplier - the supplying function that produces a value to be returned//Returns://the value, if present, otherwise the result produced by the supplying function//Throws://NullPointerException - if no value is present and the supplying function is null

Regarding the above signatures, the first difference is crystal clear: orElse accepts a generic object as an argument; however, orElseGet accepts a supplier function that produces a value. So what? You are right; it may seem negligible. However, it's not that easy. Lets check the following sample:

Optional<Integer> number = Optional.of(10);System.out.println("The number is: " + number.orElseGet(() -> alternativeNumber()));private static Integer alternativeNumber() {    int number = 747;    System.out.println("The alternative number is: " + number);    return number;}

What's the output of the above code snippet? As you see, the number is an Optional variable initiated with the value of 10. So we don't expect orElseGet to do something. Here's the output:

The number is: 10

That's super easy, right? Now check this one:

Optional<Integer> number = Optional.of(10);System.out.println("The number is: " + number.orElse(alternativeNumber()));private static Integer alternativeNumber() {    int number = 747;    System.out.println("The alternative number is: " + number);    return number;}

Everything is the same as the previous example except the second line in which we have used orElse instead of orElseGet. We don't expect the orElse to do something yet because our number has a value. Just stop here and guess the output! Same as the previous one? No! The output is:

The alternative number is: 747The number is: 10

The orElses parameter always gets evaluated; even if the Optional variable contains a value. However, the supplier method passed to the orElseGet only gets evaluated when the Optional variable is empty. Thats why the second output is different. Note that this could affect the performance drastically in cases you use orElse and pass it a complex method.

Using orElse(null)

The other common mistake I usually encounter in code reviews is somehow philosophical; that's using orElse(null). Whats wrong with it? To find the answer, let's review the motivation of Optional existence. As I previously mentioned, Optional exists to help you not to face the problems caused by null references such as NullPointerException. We also talked about the orElse method in the previous section. Now, put these two parts together. Does it make any sense to ask our Optional object to return null if its empty and doesnt contain a value?

The orElse method is a proper way to return an alternative value in the cases the Optional object is empty. Returning null, questions the usage of Optional. You may prefer to use a null reference instead of Optional; it's ok, although I never recommend it. However, mixing these two ideas is somehow misleading. At the first glance, the code seems null safe; however, you may face null reference issues, sooner or later.

Returning null in a method with Optional return type

Before describing the last and weirdest case, lets see how it happens. Lets get back to the previous example. Suppose that the findByCellNumber is something like this:

public Optional<User> findByCellNumber(String cellNumber) {    //some code here    if (someCondition) {        return null;    }    //some code here}

As you see, this method returns null in a specific condition. Suppose that the database is down and the author handles the exception and returns null in this situation (BAD PRACTICE, dont try this at home). So, there are some circumstances where you try to get the Optional value but you encounter a NullPointerException. Look at this:

User user = userDao.findByCellNumber(cellNumber).orElseThrow(UserNotFoundException::new);

We expect the method to return an Optional then we use orElseThrow; however, it returns null. So, our try leads to an exception. How to avoid this issue? This is the point: as previously mentioned, an Optional MUST not be null.

Quick Recap

• Each explicit call to get method of an Optional variable could be a code smell.
• orElse and orElseGet are not equivalent.
• Optional was added to Java to help developers avoid null references. So each return null or orElse(null) presence in your code using Optional variables is a code smell.

Keeping in mind these points, you can prevent most of the common pitfalls while using Optional.