Should Java 8 getters return optional type?

Optional type introduced in Java 8 is a new thing for many developers.

Is a getter method returning Optional<Foo> type in place of the classic Foo a good practice? Assume that the value can be null.

Solution 1:

Of course, people will do what they want. But we did have a clear intention when adding this feature, and it was not to be a general purpose Maybe type, as much as many people would have liked us to do so. Our intention was to provide a limited mechanism for library method return types where there needed to be a clear way to represent "no result", and using null for such was overwhelmingly likely to cause errors.

For example, you probably should never use it for something that returns an array of results, or a list of results; instead return an empty array or list. You should almost never use it as a field of something or a method parameter.

I think routinely using it as a return value for getters would definitely be over-use.

There's nothing wrong with Optional that it should be avoided, it's just not what many people wish it were, and accordingly we were fairly concerned about the risk of zealous over-use.

(Public service announcement: NEVER call Optional.get unless you can prove it will never be null; instead use one of the safe methods like orElse or ifPresent. In retrospect, we should have called get something like getOrElseThrowNoSuchElementException or something that made it far clearer that this was a highly dangerous method that undermined the whole purpose of Optional in the first place. Lesson learned. (UPDATE: Java 10 has Optional.orElseThrow(), which is semantically equivalent to get(), but whose name is more appropriate.))

Solution 2:

After doing a bit of research of my own, I've come across a number of things that might suggest when this is appropriate. The most authoritative being the following quote from an Oracle article:

"It is important to note that the intention of the Optional class is not to replace every single null reference. Instead, its purpose is to help design more-comprehensible APIs so that by just reading the signature of a method, you can tell whether you can expect an optional value. This forces you to actively unwrap an Optional to deal with the absence of a value." - Tired of Null Pointer Exceptions? Consider Using Java SE 8's Optional!

I also found this excerpt from Java 8 Optional: How to use it

"Optional is not meant to be used in these contexts, as it won't buy us anything:

in the domain model layer (not serializable)

in DTOs (same reason)

in input parameters of methods

in constructor parameters"

Which also seems to raise some valid points.

I wasn't able to find any negative connotations or red flags to suggest that Optional should be avoided. I think the general idea is, if it's helpful or improves the usability of your API, use it.

Solution 3:

I'd say in general its a good idea to use the optional type for return values that can be nullable. However, w.r.t. to frameworks I assume that replacing classical getters with optional types will cause a lot of trouble when working with frameworks (e.g., Hibernate) that rely on coding conventions for getters and setters.

Solution 4:

The reason Optional was added to Java is because this:

return Arrays.asList(enclosingInfo.getEnclosingClass().getDeclaredMethods())
    .stream()
    .filter(m -> Objects.equals(m.getName(), enclosingInfo.getName())
    .filter(m ->  Arrays.equals(m.getParameterTypes(), parameterClasses))
    .filter(m -> Objects.equals(m.getReturnType(), returnType))
    .findFirst()
    .getOrThrow(() -> new InternalError(...));

is cleaner than this:

Method matching =
    Arrays.asList(enclosingInfo.getEnclosingClass().getDeclaredMethods())
    .stream()
    .filter(m -> Objects.equals(m.getName(), enclosingInfo.getName())
    .filter(m ->  Arrays.equals(m.getParameterTypes(), parameterClasses))
    .filter(m -> Objects.equals(m.getReturnType(), returnType))
    .getFirst();
if (matching == null)
  throw new InternalError("Enclosing method not found");
return matching;

My point is that Optional was written to support functional programming, which was added to Java at the same time. (The example comes courtesy of a blog by Brian Goetz. A better example might use the orElse() method, since this code will throw an exception anyway, but you get the picture.)

But now, people are using Optional for a very different reason. They're using it to address a flaw in the language design. The flaw is this: There's no way to specify which of an API's parameters and return values are allowed to be null. It may be mentioned in the javadocs, but most developers don't even write javadocs for their code, and not many will check the javadocs as they write. So this leads to a lot of code that always checks for null values before using them, even though they often can't possibly be null because they were already validated repeatedly nine or ten times up the call stack.

I think there was a real thirst to solve this flaw, because so many people who saw the new Optional class assumed its purpose was to add clarity to APIs. Which is why people ask questions like "should getters return Optionals?" No, they probably shouldn't, unless you expect the getter to be used in functional programming, which is very unlikely. In fact, if you look at where Optional is used in the Java API, it's mainly in the Stream classes, which are the core of functional programming. (I haven't checked very thoroughly, but the Stream classes might be the only place they're used.)

If you do plan to use a getter in a bit of functional code, it might be a good idea to have a standard getter and a second one that returns Optional.

Oh, and if you need your class to be serializable, you should absolutely not use Optional.

Optionals are a very bad solution to the API flaw because a) they're very verbose, and b) They were never intended to solve that problem in the first place.

A much better solution to the API flaw is the Nullness Checker. This is an annotation processor that lets you specify which parameters and return values are allowed to be null by annotating them with @Nullable. This way, the compiler can scan the code and figure out if a value that can actually be null is being passed to a value where null is not allowed. By default, it assumes nothing is allowed to be null unless it's annotated so. This way, you don't have to worry about null values. Passing a null value to a parameter will result in a compiler error. Testing an object for null that can't be null produces a compiler warning. The effect of this is to change NullPointerException from a runtime error to a compile-time error.

This changes everything.

As for your getters, don't use Optional. And try to design your classes so none of the members can possibly be null. And maybe try adding the Nullness Checker to your project and declaring your getters and setter parameters @Nullable if they need it. I've only done this with new projects. It probably produces a lot of warnings in existing projects written with lots of superfluous tests for null, so it might be tough to retrofit. But it will also catch a lot of bugs. I love it. My code is much cleaner and more reliable because of it.

(There is also a new language that addresses this. Kotlin, which compiles to Java byte code, allows you to specify if an object may be null when you declare it. It's a cleaner approach.)

Addendum to Original Post (version 2)

After giving it a lot of thought, I have reluctantly come to the conclusion that it's acceptable to return Optional on one condition: That the value retrieved might actually be null. I have seen a lot of code where people routinely return Optional from getters that can't possibly return null. I see this as a very bad coding practice that only adds complexity to the code, which makes bugs more likely. But when the returned value might actually be null, go ahead and wrap it inside an Optional.

Keep in mind that methods that are designed for functional programming, and that require a function reference, will (and should) be written in two forms, one of which uses Optional. For example, Optional.map() and Optional.flatMap() both take function references. The first takes a reference to an ordinary getter, and the second takes one that returns Optional. So you're not doing anyone a favor by return an Optional where the value can't be null.

Having said all that, I still see the approach used by the Nullness Checker is the best way to deal with nulls, since they turn NullPointerExceptions from runtime bugs to compile time errors.

Solution 5:

If you are using modern serializers and other frameworks that understand Optional then I have found these guidelines work well when writing Entity beans and domain layers:

If the serialization layer (usually a DB) allows a null value for a cell in column BAR in table FOO, then the getter Foo.getBar() can return Optional indicating to the developer that this value may reasonably be expected to be null and they should handle this. If the DB guarantees the value will not be null then the getter should not wrap this in an Optional.
Foo.bar should be private and not be Optional. There's really no reason for it to be Optional if it is private.
The setter Foo.setBar(String bar) should take the type of bar and not Optional. If it's OK to use a null argument then state this in the JavaDoc comment. If it's not OK to use null an IllegalArgumentException or some appropriate business logic is, IMHO, more appropriate.
Constructors don't need Optional arguments (for reasons similar to point 3). Generally I only include arguments in the constructor that must be non-null in the serialization database.

To make the above more efficient, you might want to edit your IDE templates for generating getters and corresponding templates for toString(), equals(Obj o) etc. or use fields directly for those (most IDE generators already deal with nulls).