How to Spot When Optional is Being Used Incorrectly

Ever since Java 8 was introduced (and that was some time ago), it’s surprising how many code smells creep in by the incorrect use of Optional.

In this article I’m going to show the most common examples of this and explain how to resolve them.  To keep it simple, I will explain this without going into any of the functional concepts Optional is based on.

To demonstrate this, let’s take two classes we’ll be using to store data in: Property and Address.  A Property has many fields but the one we’ll be most interested in is the address. The Address class also has many fields for details like the road, town, etc. and when the address is in a city then it can be split down into suburbs. For this example, we will write a function to return the suburb that a property is located in, assuming that it is in one. 

A Common Problem

Let’s go back to the heady days of the early 2010s: everyone wanted a shiny new iPad, we were sad that the Harry Potter and Hunger Games films were drawing to a close, we were frightened of catching swine flu, and Java 7 was the language everyone wanted to code.

So, let’s see how this function would look in Java 7:

public String getPropertySuburb(Property property) {
    String suburb = null;
    if (property != null) {
        Address address = property.getAddress();
        if (address != null) {
            if (address.isCity()) {
                suburb = address.getSuburb();
            }
        }
    }
    return suburb;
}

The first thing you’ll notice are the multiple levels of indentation. Here we are just looking for a suburb in an address in a property, but you’ll notice that each step needs its own checks for obejcts being null. This can get quite hard to read (and write) if we have a lot of nested objects.

Because of the amount of nesting, multiple returns were often quite hard to spot, so having a single return in this manner was considered a good practice.

Once Optional came along, it was common to use it in the following way. I have split the logic for getting the suburb from address into a separate function – I will come back to this later.

public String getPropertySuburb(Property property) {
    String suburb = null;
    Optional<Property> propertyOptional = Optional.ofNullable(property);
    if (propertyOptional.isPresent()) {
        Address address = propertyOptional.get().getAddress();
        suburb = getAddressSuburb(address);
    }
    return suburb;
}
 
private String getAddressSuburb(Address address) {
    String suburb = null;
    Optional<Address> addressOptional = Optional.ofNullable(address);
    if (addressOptional.isPresent()) {
        if (address.isCity()) {
            suburb = address.getSuburb();
        }
    }
    return suburb;
}

Mapping the Optional Value

When I first saw code like this, I was convinced that there was a mistake here! We have moved the null check into a check for Optional being present, but this will only be the case when the object was not null in the first place. It is the same logic only it’s not so clear (not explicit). What is the point of using Optional if it doesn’t provide any benefits?

The problem is that using Optional.ispresent() and Optional.get() in this way is not adding value and is an indicator that the code can be improved.

public String getPropertySuburb(Property property) {
    String suburb = null;
    Optional<Property> propertyOptional = Optional.ofNullable(property);
    Address address = propertyOptional.map(property -> property.getAddress()).orElse(null);
    suburb = getAddressSuburb(address);
    return suburb;
}

I’ve kept it in a similar format to the previous function and on the face of it it doesn’t look much better. There is a still a lot of boilerplate code. But there are simple things we can do to improve it:

First up, we can remove a lot of the variables. The only place where we use propertyOptional is in address so we can inline it in the address creation. Because the map() function returns a new Optional, it allows us to chain multiple map() calls together. So, we can go one step further because suburb is just calling a function to map the address. If we apply this mapping before calling orElse, then we guarantee it will only be called with a non null value which will simplify that function. This means we can create a new suburb and then return it so we can make one final improvement by returning the value directly.

Secondly, Java allows us to replace common function formats, so foo → foo.bar()  can be written as Foo::bar (where Foo is foo‘s class or anancestor), and foo → bar.zot(foo)  can be replaced by bar::zot . This means we can map not just getting the address but also getting the suburb. If you find it hard to remember which is which, don’t worry, modern IDEs will happily point out situations where you can make these changes.

So now, we can get it down to this:

public String getPropertySuburb(Property property) {
    return Optional.ofNullable(property)
            .map(Property::getAddress)
            .map(this::getAddressSuburb)
            .orElse(null);
}

This is a lot simpler and easier to read: we take the property, we get the address from the property, and then pass this on to a function to get the suburb. And if any of this fails, we’ll return null. Now this in itself seems a bit odd: we already have an Optional which is only set when we have a valid value, so surely it makes sense to keep it as Optional?

public Optional<String> getPropertySuburb(Property property) {
    return Optional.ofNullable(property)
            .map(Property::getAddress)
            .map(this::getAddressSuburb);
}

This highlights an important thing that Optional is doing; the function should return a suburb for the property, but if we:

• Don’t pass in a property (it is null),
• or pass in a property without an address,
• or pass in a property with an address which is not in a city,
• or pass in a property with an address in a city but without a suburb,

then we cannot return a suburb and the function will fail. Returning an Optional tells the calling function that it is possible to pass in parameters which cannot return a valid response, and:

• it can continue processing as though there wasn’t a problem (with map() above and also filter() and flatMap()),
• it can set a default value for suburb (with orElse and several related functions).

The caller doesn’t need to perform explicit checking and it keeps control of the code. This second point is important because you could get similar behaviour by throwing an exception, but if the calling function doesn’t know which exceptions can be thrown (and bear in mind this could be thrown by a function deep in the call stack), then this could easily abort the caller’s execution.

Testing the Optional Value

Now let’s go back to the getAddressLocation() function. We know that it is now only being called when the address exsts. So, we can now have the function just return the suburb if it exists.

Let’s start by updating our current code:

private String getAddressSuburb(Address address) {
    if (!address.isCity()) {
        return null;
    }
    return address.getSuburb();
}

So the first thing is, we’re now returning null when the parameter might not produce a valid response, and we know that using Optional would be better here:

private Optional<String> getAddressSuburb(Address address) {
    if (!address.isCity()) {
        return Optional.empty();
    }
    return Optional.ofNullable(address.getSuburb());
}

But we could remove the if block. Again, Optional comes to the fore with another function: Optional::filter.

private Optional<String> getAddressSuburb(Address address) {
    return Optional.of(address)
        .filter(Address::isCity)
        .map(Address::getSuburb);
}

Both of these versions of getAdressSuburb() work in different ways to do the same thing, but that doesn’t mean you should always prefer one over the other. Above, I mentioned that Optionals are used to indicate that a parameter may not be valid, and an easy way to do this is to put a simple check at the top of the function for bad parameters so that the rest of the function can concentrate on valid processing. This is referred to as a “guard” as it protects the core logic from invalid parameters. So, both of the functions above are equally good (one with a guard and one with a filter), and it is up to the engineer to decide which is clearer. I will continue to use the filter example.

Mapping an Optional to Another Optional

Unfortunately, this breaks the build! The problem is that Optional::map returns an Optional containing the response of getAddressSuburb(), and this is now another Optional! Fortunately, Optional provides us with another mapping function in this circumstance: Optional::flatMap.

public Optional<String> getPropertySuburb(Property property) {
    return Optional.ofNullable(property)
        .map(Property::getAddress)
        .flatMap(this::getAddressSuburb);
}
 
private Optional<String> getAddressSuburb(Address address) {
    return Optional.of(address)
        .filter(Address::isCity)
        .map(Address::getSuburb);
}

Knowing the difference between map() and flatMap() can be hard to get right, so as a rule of thumb, if the conversion function doesn’t return an Optional you will want to use map(). If it returns an Optional then you probably want to use flatMap().

Conclusion

In this article, I have shown you how Optional should not be used as a like-for-like replacement for null checking but rather as a way to handle parameters which would otherwise throw exceptions.

This is done by the use of map(), filter(), and flatMap() to manipulate the value in the Optional, as well as guards to return null when the parameter is invalid.

Hopefully this will help you to decide when and how to use Optional in your own code.