Deprecating Optional.get()

Written by (royvanrijn.com) on

Deprecating Optional.get()

On the OpenJDK core mailinglist (and Twitter) there is a discussion about Java’s Optional. Before diving into that discussion, lets take a look at what Optional does and how you can use it.

Checking for null

What do you do when your code calls an external service or god forbid a microservice, and the result isn’t always available?

Most of the time the protocol you are using facilitates in the optional part, for example in REST you’ll get a 404 instead of JSON. Getting this 404 forces you to think about this scenario and do something when this happens.

But what do you do when you’re calling a framework (on the boundary of your code) and the value isn’t always known?

You either get the value or the result is a dreaded null. This causes a lot of null checks, or bugs where the code just crashes with a NullPointerException.

Example (old skool, Java 7):

    Order order = Database.readOrder(); //can be null
    if(order != null) {
        ProcessResult result = OrderEngine.process(order);
        if(result != null && result.succeeded()) {
            Database.storeResult(result);
        }
    }

This code is not very pleasant to read. But we Java programmers didn’t have or need anything better… until we started to adopt a more functional style of programming.

java.util.Optional

What happens when you are processing a stream and some values are null? You don’t want null checks inside a stream! This is where Java 8’s Optional comes in. If you’re not (yet!?) using Java 8, there are other implementations as well. For example Google Guava has an Optional as well.

Optional is a class that ‘might’ have a given value in it, or not, it is optional. So how exactly is this helpful? Instead of checking for null this wrapper class can handle some situations for you.

For example:

    Optional<Order> order = Database.readOrder();
    Optional<ProcessResult> result = order.map(OrderEngine::process);
    Optional<ProcessResult> filteredResult = result.filter(ProcessResult::succeeded);

    filteredResult.ifPresent(Database::storeResult);

Or the shorter ‘fluid’ version:

    Database.readOrder()
        .map(OrderEngine::process)
        .filter(ProcessResult::succeeded)
        .ifPresent(Database::storeResult);

Even if the Optional is empty in either reading the order or processing the order… nothing breaks. No NullPointerException, nothing, just no executed lambda storing the result in the end. We’ve eliminated the need for a null check.

As you can see Optional can really clean up your code. You don’t need to worry about null checks anymore.

So what is the problem with Optional.get()?

Optional.get() deprecation discussion

Pretty much out of the blue on the OpenJDK mailinglist an email arrived with a webrev (similar to a patch file) that contained the deprecation of Optional.get().

The get() method is too easy to find, and the name isn’t quite what you’d expect, and the webrev author claims there are a lot of cases online where people made the same mistakes.

Many programmers, when they first encounter Optional, don’t know what to do. They look in their IDE and the first thing that pops up is get().

It is just an easy method to call:

    Order order = Database.readOrder().get();

This works fine! Until there is a situation where the value is not available to the Optional. In that case it will throw an NoSuchElementException. How can we solve this? Well, we could do the following:

    Optional<Order> maybeOrder = Database.readOrder();
    if(maybeOrder.isPresent()) {
        Order order = maybeOrder.get();
    }

This is the ‘safe’ way, but it could just as well have been a null check now. There is likely a much cleaner way to process your Optional.

  • If you want to do something with the result, use filter, map, ifPresent (and many others).
  • If you need to return something, either return an Optional yourself, or get a default value by calling orElse, orElseGet or orElseThrow.

This is all you need, why have a get-method?

The proposal on the mailinglist is to deprecate the get() method and rename it to getWhenPresent(). This name change should warn people that it might not be present and they should check isPresent before calling get().

Instead of embracing this change some people on the mailinglist argue against deprecation, some of their reasons:

  • Renaming will break a LOT of code, well, not really break the code, it will cause deprecation warnings
  • getWhenPresent() instead of get() just adds noise to the code, it doesn’t solve anything
  • People should just read the JavaDoc, it clearly states what get() does and throws
  • Guava’s Optional also has the same get() method, they’ve never heard about the problem

The most honest and one of the more powerful replies in the discussion was from Brian Goetz himself:

As the person who chose the original (terrible) name, let me weigh in…

……

I’d like to see it fixed, and the sooner the better.

He is clearly in favor of deprecation… what is your opinion? Let me know in the comments!