A Modest Critique of Optional Handling

by: Ethan McCue

java.util.Optional is a class that breaks peoples brains a bit.

It offers a way to represent a potentially missing piece of information. If your method might not produce a result you can use an empty Optional to represent that.

Optional<LocalDate> birthday(String name) {
    return switch (name) {
        case "Mr. Rodgers" -> 
                Optional.of(LocalDate.of(1928, Month.MARCH, 20));
        case "Mr. T" -> 
                Optional.of(LocalDate.of(1952, Month.MAY, 21));
        default -> 
                Optional.empty();
    };
}

void main() {
    for (var name : List.of("Mr. T", "H.R. Puffinstuff")) {
        IO.println(name + ": " + birthday(name));
    }
}

This has a few pros over the other standard option of returning null.

For one, Java's type system (currently) does not track null values as part of a type. This means that a programmer needs to infer nullability from written documentation and context clues.

LocalDate birthday(String name) {
    return switch (name) {
        case "Mr. Rodgers" -> 
                LocalDate.of(1928, Month.MARCH, 20);
        case "Mr. T" -> 
                LocalDate.of(1952, Month.MAY, 21);
        default -> 
                null;
    };
}

void main() {
    for (var name : List.of("Mr. T", "H.R. Puffinstuff")) {
        // .getMonth() is not always valid to call
        IO.println(name + ": " + birthday(name).getMonth());
    }
}

This is mitigated somewhat when an API makes use of nullability annotations. Today that requires you to use special null-aware tooling but null-aware types are slated to come to Java proper at some point.

// LocalDate? in the future (probably)
@Nullable LocalDate birthday(String name) {
    return switch (name) {
        case "Mr. Rodgers" -> 
                LocalDate.of(1928, Month.MARCH, 20);
        case "Mr. T" -> 
                LocalDate.of(1952, Month.MAY, 21);
        default -> 
                null;
    };
}

void main() {
    for (var name : List.of("Mr. T", "H.R. Puffinstuff")) {
        // Tooling should catch this mistake
        IO.println(name + ": " + birthday(name).getMonth());
    }
}

But null values in Java always require explicit checks to handle.

if (v == null) {
    // ...
}

// or

v == null ? ... : ...;

// or Objects.requireNonNullElse(v, ...)
// but that's just a method

This makes them ill-suited to the task Optional was originally introduced for - being part of the customary chain of method calls that form stream operations. This is most important when an Optional appears in the middle of a stack of such operations, but even when the Optional handling is at the end it's at least aesthetic.

Stream.of(1, 2, 3)
    .filter(x -> x < 2)
    .findFirst() // Here is where we get an optional
    .orElse(0); // And we can preserve the method chain stack

People are far more likely to remember to write the above when Java forces them to. That's the point of it.

Integer first = Stream.of(1, 2, 3)
    .filter(x -> x < 2)
    .findFirst(); 
// Not forced to handle null usually and it's aesthetically
// inconvenient. This leads to mistakes in the context of chained methods.
if (first == null) {
    first = 0;
}

But because it "solves null" (it doesn't) it sees a lot of use. A very common pattern has been for folks to take a null returning method and refactor it to use optional instead.

When this works its generally fine, what isn't fine is how callsites are often adapted.

User user = findUserById(id);
if (user != null) {
    // Logic
}

The code above is often turned into something like the following.

Optional<User> userOpt = findUserById(id);
if (userOpt.isPresent()) {
    User user = userOpt.get(); // or .orElseThrow() if civilized
    // Logic
}

Callsites refactored like this are strictly worse. Often an extra intermediate variable needs to be created, that intermediate has a stupid name, and static analysis tools will either not know what is happening or have special carve out exceptions so they know "when .isPresent(), .get() is okay."

Those carve out exceptions may or may not also apply to other ecosystem Optional types.

// It's hard to argue that java.util.Optional really deserves
// special treatment over vavr's Option, but because code like
// this exists it's going to get it.
Option<User> userOpt = findUserById(id);
if (userOpt.isDefined()) {
    User user = userOpt.get();
    // Logic
}

And the general sort of advice to deal with this is to use .map and .ifPresent - the methods that let you work with the value in an Optional without "unboxing" it.

findUserById(id).ifPresent(user -> {
    // Logic
});

// or keep chaining and deal with it later

findUserById(id).map(User::email);

When this works, it can be better. It just stops working when the code you want to run that involves the value might throw a checked exception - .ifPresent and friends cannot cope with that.

findUserById(id).ifPresent(user -> {
    // Logic
    if (...) {
        // Difficult to know what to do.
        // Do we wrap the exception?
        callRemoteAPI();  
    }
    // Logic
});

It also stops working when you have multiple Optionals, at least from a readability perspective.

findUserById(idA).ifPresent(userA -> {
    findUserById(idB).ifPresent(userB -> {
        findUserById(idC).ifPresent(userC -> {
            // Logic
        }); 
    });
});

And also when you want to mutate some local variables relevant to the rest of the function.

int existingUsers = 0;
findUserById(id).ifPresent(user -> {
    // Logic
    existingUsers++; // Lambda rules - does not work
    // Logic
});

And just in general, beyond the small and clean usages, I think using these methods makes code net worse.

What I suggest, and I think people don't consider specifically because they are viewing Optional as a replacement for null, is to use .orElse(null) and write the rest of the code in the method like normal.

User user = findUserById(id).orElse(null);
if (user != null) {
    // Logic
}

Static analysis tools can eat "local variable known to sometimes be null" for breakfast and there are no problems handling checked exceptions, multiple optionals, or mutating local state. It's a much more seamless refactor.

int existingUsers = 0;

User userA = findUserById(idA).orElse(null);
User userB = findUserById(idB).orElse(null);
User userC = findUserById(idC).orElse(null);
if (userA != null && userB != null && userC != null) {
    // Logic
    existingUsers++;
    if (...) {
        callRemoteAPI();  
    }
    // Logic
}

The fact that it might be null also becomes explicit in the source code at every usage site, which is a strict improvement over the equivalent code with null returning methods.

int existingUsers = 0;

// Back to inferring from context or documentation
User userA = findUserById(idA);
User userB = findUserById(idB);
User userC = findUserById(idC);
if (userA != null && userB != null && userC != null) {
    // Logic
    existingUsers++;
    if (...) {
        callRemoteAPI();  
    }
    // Logic
}

There are more bike sheds to have on Optional, don't get me wrong, I just want to strongly encourage you to reconsider blanket advice to "avoid .isPresent/.get, use .map/.ifPresent" and highlight that .orElse is useful for more than valid default values.

<- Index