106

We have recently moved to Java 8. Now, I see applications flooded with Optional objects.

Before Java 8 (Style 1)

Employee employee = employeeServive.getEmployee();

if(employee!=null){
    System.out.println(employee.getId());
}

After Java 8 (Style 2)

Optional<Employee> employeeOptional = Optional.ofNullable(employeeService.getEmployee());
if(employeeOptional.isPresent()){
    Employee employee = employeeOptional.get();
    System.out.println(employee.getId());
}

I see no added value of Optional<Employee> employeeOptional = employeeService.getEmployee(); when the service itself returns optional:

Coming from a Java 6 background, I see Style 1 as more clear and with fewer lines of code. Is there any real advantage I am missing here?

Consolidated from understanding from all answers and further research at blog

  • 19
    Oracle has an extensive article about the use of Optional. – Mike Partridge Jan 18 '18 at 15:33
  • 27
    Yuck! employeeOptional.isPresent() seems to be entirely missing the point of option types. As per @MikePartridge's comment, this isn't recommended or idiomatic. Explicitly checking whether an option type is something or nothing is a no-no. You're supposed to simply map or flatMap over them. – Andres F. Jan 18 '18 at 19:43
  • 6
    See a Stack Overflow Answer on Optional by Brian Goetz, the Java Language Architect at Oracle. – Basil Bourque Jan 19 '18 at 2:17
  • 5
    This is what happens when you turn off your brain in the name of "best practices". – immibis Jan 19 '18 at 4:41
  • 9
    That is fairly poor usage of optional but even that has benefits. With nulls everything may be null, so you need to constantly be checking, optional calls out that this particular variable is likely to be missing, make sure you check for it – Richard Tingle Jan 19 '18 at 6:58

10 Answers 10

107

Style 2 isn't going Java 8 enough to see the full benefit. You don't want the if ... use at all. See Oracle's examples. Taking their advice, we get:

Style 3

// Changed EmployeeServive to return an optional, no more nulls!
Optional<Employee> employee = employeeServive.getEmployee();
employee.ifPresent(e -> System.out.println(e.getId()));

Or a more lengthy snippet

Optional<Employee> employee = employeeServive.getEmployee();
// Sometimes an Employee has forgotten to write an up-to-date timesheet
Optional<Timesheet> timesheet = employee.flatMap(Employee::askForCurrentTimesheet); 
// We don't want to do the heavyweight action of creating a new estimate if it will just be discarded
client.bill(timesheet.orElseGet(EstimatedTimesheet::new));
  • 17
    indeed we ban most uses of isPresent (caught by code review) – jk. Jan 18 '18 at 16:05
  • 10
    @jk. indeed, if there were an overload void ifPresent(Consumer<T>, Runnable) I would argue for a total ban of isPresent and get – Caleth Jan 18 '18 at 16:23
  • 10
    @Caleth: You may be interested in Java 9's ifPresentOrElse(Consumer<? super T>, Runnable). – wchargin Jan 19 '18 at 4:47
  • 9
    I find one drawback in this java 8 style, compared to the java 6 one : it fools code coverage tools. With if statement, it shows when you missed a branch, not here. – Thierry Jan 19 '18 at 9:10
  • 14
    @Aaron "drastically reduces code readability". What part exactly reduces readability? Sounds more like a "I've always done it differently and I don't want to change my habits" than objective reasoning. – Voo Jan 20 '18 at 12:35
45

If you're using Optional as a "compatibility" layer between an older API that may still return null, it may be helpful to create the (non-empty) Optional at the latest stage that you're sure that you have something. E.g., where you wrote:

Optional<Employee> employeeOptional = Optional.ofNullable(employeeService.getEmployee());
if(employeeOptional.isPresent()){
    Employee employeeOptional= employeeOptional.get();
    System.out.println(employee.getId());
}

I'd opt toward:

Optional.of(employeeService)                 // definitely have the service
        .map(EmployeeService::getEmployee)   // getEmployee() might return null
        .map(Employee::getId)                // get ID from employee if there is one
        .ifPresent(System.out::println);     // and if there is an ID, print it

The point is that you know that there's a non-null employee service, so you can wrap that up in an Optional with Optional.of(). Then, when you call getEmployee() on that, you may or may not get an employee. That employee may (or, possibly, may not) have an ID. Then, if you ended up with an ID, you want to print it.

There's no need to explicitly check for any null, presence, etc., in this code.

  • 4
    What is the advantage of using (...).ifPresent(aMethod) over if((...).isPresent()) { aMethod(...); }? It seems to simply be yet another syntax to do almost the same operation. – Ruslan Jan 20 '18 at 17:39
  • 2
    @Ruslan Why use one special syntax for one specific call when you already have a general solution that works equally well for all cases? We're just applying the same idea from map and co here as well. – Voo Jan 20 '18 at 17:44
  • 1
    @Ruslan ... an API returns nulls instead of empty collections or arrays. E.g., you can do something like (assuming a Parent class for which getChildren() returns a set of children, or null if there are no children): Set<Children> children = Optional.of(parent).map(Parent::getChildren).orElseGet(Collections::emptySet); Now I can process children uniformly, e.g., children.forEach(relative::sendGift);. Those could even be combined:Optional.of(parent).map(Parent::getChildren).orElseGet(Collections::emptySet).forEach(relative::sendGift);. All the boilerplate of null checking, etc., ... – Joshua Taylor Jan 20 '18 at 17:55
  • 1
    @Ruslan ... is delegated to tried and true code in the standard library. That reduces the amount of code that I, as a programmer, have to write, test, debug, etc. – Joshua Taylor Jan 20 '18 at 17:56
  • 1
    Glad I'm using Swift. if let employee = employeeService.employee { print (employee.Id); } – gnasher729 Jan 20 '18 at 20:01
23

There is next to no added value in having an Optional of a single value. As you've seen, that just replaces a check for null with a check for presence.

There is huge added value in having a Collection of such things. All the streaming methods introduced to replace old-style low-level loops understand Optional things and do the right thing about them (not processing them or ultimately returning another unset Optional). If you deal with n items more often than with individual items (and 99% of all realistic programs do), then it is a huge improvement to have built-in support for optionally present things. In the ideal case you never have to check for null or presence.

  • 24
    Aside from collections/streams, an Optional is also great to make APIs more self-documenting, e.g. Employee getEmployee() vs Optional<Employee> getEmployee(). While technically both could return null, one of them is far more likely to cause trouble. – amon Jan 18 '18 at 15:01
  • 16
    There's plenty of value in an optional of a single value. Being able to .map() without having to check for null all along the way is great. E.g., Optional.of(service).map(Service::getEmployee).map(Employee::getId).ifPresent(this::submitIdForReview);. – Joshua Taylor Jan 18 '18 at 19:41
  • 9
    I disagree with your initial comment of no added value. Optional provides context to your code. A quick glance can tell you correct to correctness of a function and all cases are covered. Whereas with null check, should you null check every single Reference type on every single method? A similar case can be applied to Classes and public/private modifiers; they add zero value to your code, right? – ArTs Jan 19 '18 at 4:18
  • 3
    @JoshuaTaylor Honestly, compared to that expression, I prefer the old-fashioned check for null. – Kilian Foth Jan 19 '18 at 9:41
  • 2
    Another big advantage of Optional even with single values is that you cannot forget to check for null when you should do. Otherwise it is very easy to forget the check and end up with a NullPointerException... – Sean Burton Jan 19 '18 at 15:35
17

As long as you use Optional just like a fancy API for isNotNull(), then yes, you won't find any differences with just checking for null.

Things you should NOT use Optional for

Using Optional just to check for the presence of a value is Bad Code™:

// BAD CODE ™ --  just check getEmployee() != null  
Optional<Employee> employeeOptional =  Optional.ofNullable(employeeService.getEmployee());  
if(employeeOptional.isPresent()) {  
    Employee employee = employeeOptional.get();  
    System.out.println(employee.getId());
}  

Things you should use Optional for

Avoid a value not being present, by producing one when necessary when using null-returning API methods:

Employee employee = Optional.ofNullable(employeeService.getEmployee())
                            .orElseGet(Employee::new);
System.out.println(employee.getId());

Or, if creating a new Employee is too costly:

Optional<Employee> employee = Optional.ofNullable(employeeService.getEmployee());
System.out.println(employee.map(Employee::getId).orElse("No employee found"));

Also, making everybody aware that your methods might not return a value (if, for whatever reason, you cannot return a default value like above):

// Your code without Optional
public Employee getEmployee() {
    return someCondition ? null : someEmployee;
}
// Someone else's code
Employee employee = getEmployee(); // compiler doesn't complain
// employee.get...() -> NPE awaiting to happen, devs criticizing your code

// Your code with Optional
public Optional<Employee> getEmployee() {
    return someCondition ? Optional.empty() : Optional.of(someEmployee);
}
// Someone else's code
Employee employee = getEmployee(); // compiler complains about incompatible types
// Now devs either declare Optional<Employee>, or just do employee = getEmployee().get(), but do so consciously -- not your fault.

And finally, there's all the stream-like methods that have already been explained in other answers, though those are not really you deciding to use Optional but making use of the Optional provided by someone else.

  • 1
    @Kapep Indeed. We had this debate over at /r/java, and I said: «I see Optional as a missed opportunity. "Here, have this new Optional thing I made so you are forced to check for null values. Oh, and by the way... I added a get() method to it so you don't actually have to check for anything ;) ;)". (...) Optional is nice as a "THIS MIGHT BE NULL" neon sign, but the bare existence of its get() method cripples it». But OP was asking for reasons to use Optional, not reasons against it or design flaws. – walen Jan 19 '18 at 9:40
  • 1
    Employee employee = Optional.ofNullable(employeeService.getEmployee()); In your third snippet, shouldn't the type be Optional<Employee>? – Charlie Harding Jan 19 '18 at 12:55
  • 2
    @immibis In what way crippled? The main reason to use get is if you have to interact with some legacy code. I can't think of many valid reasons in new code to ever use get. That said when interacting with legacy code there's often no alternative, but still walen has a point that the existence of get causes beginners to write bad code. – Voo Jan 20 '18 at 12:48
  • 1
    @immibis I didn't mention Map once and I'm not aware of anyone making such a drastic non-backwards compatible change, so sure you could intentionally design bad APIs with Optional - not sure what that proves. An Optional would make sense for some tryGet method though. – Voo Jan 25 '18 at 9:42
  • 2
    @Voo Map is an example everyone is familiar with. Of course they can't make Map.get return an Optional because of backwards compatibility, but you can easily imagine another Map-like class that wouldn't have such compatibility constraints. Say class UserRepository {Optional<User> getUserDetails(int userID) {...}}. Now you want to get the logged in user's details, and you know they exist because they managed to log in and your system never deletes users. So you do theUserRepository.getUserDetails(loggedInUserID).get(). – immibis Jan 25 '18 at 21:29
12

The key point for me in using Optional is keep clear when developer need check if function return value or not.

//service 1
Optional<Employee> employeeOptional = employeeService.getEmployee();
if(employeeOptional.isPresent()){
    Employee employeeOptional= employeeOptional.get();
    System.out.println(employee.getId());
}

//service 2
Employee employee = employeeService.getEmployeeXTPO();
System.out.println(employee.getId());
  • 8
    It baffles me that all the nifty functional stuff with optionals, although really nice to have, is considered more important than this point right here. Before java 8, you had to dig through Javadoc to know if you had to null-check the response from a call. Post java 8, you know from the return type if you can get "nothing" back or not. – Buhb Jan 19 '18 at 7:02
  • 3
    Well, there is the "old code" problem where things still could return null... but yes, being able to tell from the signature is great. – Haakon Løtveit Jan 19 '18 at 13:50
  • 5
    Yeah it certainly works better in languages where optional<T> is the only way to express that a value might be missing, i.e. languages without null ptr. – dureuill Jan 20 '18 at 10:03
8

Your main mistake is that you're still thinking in more procedural terms. This is not meant as a criticism of you as a person, it's merely an observation. Thinking in more functional terms comes with time and practice, and therefore the methods isPresent and get look like the most obvious correct things to call for you. Your secondary minor mistake is creating your Optional inside your method. The Optional is meant to help document that something may or may not return a value. You might get nothing.

This has led you do write perfectly legible code that seems perfectly reasonable, but you were seduced by the vile twin temptresses that are get and isPresent.

Of course the question quickly becomes "why are isPresent and get even there?"

A thing that many people here miss is that isPresent() is that it's not something that is made for new code written by people fully on board with how gosh-darn useful lambdas are, and who like the functional thing.

It does however, give us a few (two) good, great, glamourous(?) benefits:

  • It eases transition of legacy code to use the new features.
  • It eases the learning curves of Optional.

The first one is rather simple.

Imagine you have an API that looked like this:

public interface SnickersCounter {
  /** 
   * Provides a proper count of how many snickers have been consumed in total.
   */
  public SnickersCount howManySnickersHaveBeenEaten();

  /**
    * returns the last snickers eaten.<br>
    * If no snickers have been eaten null is returned for contrived reasons.
    */
  public Snickers lastConsumedSnickers();
}

And you had a legacy class using this as such (fill in the blanks):

Snickers lastSnickers = snickersCounter.lastConsumedSnickers();
if(null == lastSnickers) {
  throw new NoSuchSnickersException();
}
else {
  consumer.giveDiabetes(lastSnickers);
}

A contrived example to be sure. But bear with me here.

Java 8 has now launched and we're scrambling to get aboard. So one of the things we do is that we want to replace our old interface with something that returns Optional. Why? Because as someone else has already graciously mentioned: This takes the guesswork out of whether or not something can be null This has already been pointed out by others. But now we have a problem. Imagine we have (excuse me while I hit alt+F7 on an innocent method), 46 places where this method is called in well tested legacy code that does an excellent job otherwise. Now you have to update all of these.

THIS is where isPresent shines.

Because now: Snickers lastSnickers = snickersCounter.lastConsumedSnickers(); if(null == lastSnickers) { throw new NoSuchSnickersException(); } else { consumer.giveDiabetes(lastSnickers); }

becomes:

Optional<Snickers> lastSnickers = snickersCounter.lastConsumedSnickers();
if(!lastSnickers.isPresent()) {
  throw new NoSuchSnickersException();
}
else {
  consumer.giveDiabetes(lastSnickers.get());
}

And this is a simple change you can give the new junior: He can do something useful, and he'll get to explore the codebase at the same time. win-win. After all, something akin to this pattern is pretty widespread. And now you don't have to rewrite the code to use lambdas or anything. (In this particular case it would be trivial, but I leave thinking up examples where it would be difficult as an exercise to the reader.)

Notice that this means that the way you did it is essentially a way to deal with legacy code without doing costly rewrites. So what about new code?

Well, in your case, where you just want to print something out, you'd simply do:

snickersCounter.lastConsumedSnickers().ifPresent(System.out::println);

Which is pretty simple, and perfectly clear. The point that's slowly bubbling up to the surface then, is that there exist use cases for get() and isPresent(). They're there for letting you modify existing code mechanically to use the newer types without thinking too much about it. What you're doing is therefore misguided in the following ways:

  • You're calling a method that may return null. The correct idea would be that the method returns null.
  • You're using the legacy bandaid methods to deal with this optional, instead of using the tasty new methods that contain lambda fanciness.

If you do want to use Optional as a simple null-safety check, what you should have done is simply this:

new Optional.ofNullable(employeeServive.getEmployee())
    .map(Employee::getId)
    .ifPresent(System.out::println);

Of course, the good looking version of this looks like:

employeeService.getEmployee()
    .map(Employee::getId)
    .ifPresent(System.out::println);

By the way, while it is by no means required, I do recommend using a new line per operation, so that it's easier to read. Easy to read and understand beats conciseness any day of the week.

This is of course a very simple example where it's easy to understand everything that we're trying to do. It's not always this simple in real life. But notice how in this example, what we're expressing is our intentions. We want to GET the employee, GET his ID, and if possible, print it. This is the second big win with Optional. It allows us to create clearer code. I also do think that doing things like making a method that does a bunch of stuff so that you can feed it to a map is in general a good idea.

  • 1
    The problem here is that you try to make it sound like these functional versions are just as readable as the older versions, in one case even saying an example was "perfectly clear." That is not the case. That example was clear, but not perfectly so, as it requires more thought. map(x).ifPresent(f) simply does not make the same kind of intuitive sense that if(o != null) f(x) does. The if version is perfectly clear. This is another case of people jumping on a popular band-wagon, *not* a case of real progress. – Aaron Jan 19 '18 at 20:36
  • 1
    Note that I am not saying there is zero benefit in the use of functional programming or of Optional. I was referring only to the use of optional in this specific case, and more-so to the readability, since multiple people are acting like this format is equally or more readable than if. – Aaron Jan 19 '18 at 20:40
  • 1
    It depends on our notions of clarity. While you do make very good points, I would like to also point out that to many people lambdas were new and strange at some point. I would dare wager an internet cookie that quite a few people say isPresent and get and felt delightful relief: They could now get on with updating the legacy code and then learn the lambda syntax later. On the other hand, people with Lisp, Haskell or similar language experience were probably delighted that they could now apply familiar patterns to their problems in java... – Haakon Løtveit Jan 22 '18 at 12:00
  • @Aaron - clarity isn't the point of Optional types. I personally find that they don't decrease clarity, and there are some cases (albeit not this case) where they do increase it, but the key benefit is that (as long as you avoid using isPresent and get as much as is realistically possible) they improve safety. It is harder to write incorrect code that fails to check for absence of a value if the type is Optional<Whatever> rather than using a nullable Whatever. – Jules Mar 5 '18 at 16:52
  • Even if you immediately take the values out, as long as you don't use get, you're forced to choose what to do when a missing value occurs: you would either use orElse() (or orElseGet()) to supply a default, or orElseThrow() to throw a chosen exception that documents the nature of the problem, which is better than a generic NPE that is meaningless until you dig into the stack trace. – Jules Mar 5 '18 at 16:52
0

I don't see benefit in Style 2. You still need to realize that you need the null check and now it's even larger, thus less readable.

Better style would be, if employeeServive.getEmployee() would return Optional and then the code would become:

Optional<Employee> employeeOptional = employeeServive.getEmployee();
if(employeeOptional.isPresent()){
    Employee employee= employeeOptional.get();
    System.out.println(employee.getId());
}

This way you cannot callemployeeServive.getEmployee().getId() and you notice that you should handle optional valoue somehow. And if in your whole codebase methods never return null (or almost never with well known to your team exceptions to this rule), it will increase safety against NPE.

  • 12
    Optional becomes a pointless complication the moment you use isPresent(). We use optional so we don't have to test. – candied_orange Jan 18 '18 at 15:53
  • 2
    Like others have said, do not use isPresent(). This is the wrong approach to optionals. Use map or flatMap instead. – Andres F. Jan 18 '18 at 19:45
  • 1
    @CandiedOrange And yet the top-voted answer (saying to use ifPresent) is just another way to test. – immibis Jan 19 '18 at 4:44
  • 1
    @CandiedOrange ifPresent(x) is just as much of a test as if(present) {x}. The return value is irrelevant. – immibis Jan 19 '18 at 4:58
  • 1
    @CandiedOrange Nonetheless, you originally said "so we don't have to test." You can't say "so we don't have to test" when you are, in fact, still testing... which you are. – Aaron Jan 19 '18 at 20:44
0

I think the biggest benefit is that if your method signature returns an Employee you don't check for null. You know by that signature that it's guaranteed to return an employee. You don't need to handle a failure case. With an optional you know you do.

In code I've seen there are null checks that will never fail since people don't want to trace through the code to determine if a null is possible, so they throw null checks everywhere defensively. This makes the code a bit slower, but more importantly it makes the code much noisier.

However, for this to work, you need to apply this pattern consistently.

  • 1
    The simpler way to achieve this is using @Nullable and @ReturnValuesAreNonnullByDefault together with static analysis. – maaartinus Jan 19 '18 at 8:07
  • @maaartinus Adding additional tooling in the form of static analysis and documentation in the decorator annotations, is simpler then compile time API support? – ArtB Jan 23 '18 at 4:14
  • Adding additional tooling indeed is sort of simpler, as it requires no code changes. Moreover, you want to have anyway as it catches other bugs, too. +++ I wrote a trivial script adding package-info.java with @ReturnValuesAreNonnullByDefault to all my packages, so all I have to do is to add @Nullable where you have to switch to Optional. This means fewer chars, no added garbage and no runtime-overhead. I don't have to look up the documentation as it shows when hovering over the method. The static analysis tool catches mistakes and later nullability changes. – maaartinus Jan 23 '18 at 19:45
  • My biggest concern with Optional is that it adds an alternative way of dealing with nullability. If it was in Java since the very beginning, it may be fine, but now it's just a pain. Going the Kotlin way would be IMHO much better. +++ Note that List<@Nullable String> is still a list of strings, while List<Optional<String>> is not. – maaartinus Jan 23 '18 at 19:47
0

My answer is: Just don't. At least think twice about whether it's really an improvement.

An expression (taken from another answer) like

Optional.of(employeeService)                 // definitely have the service
        .map(EmployeeService::getEmployee)   // getEmployee() might return null
        .map(Employee::getId)                // get ID from employee if there is one
        .ifPresent(System.out::println);     // and if there is an ID, print it

seems to be the right functional way of dealing with originally nullable returning methods. It looks nice, it never throws and it never makes you think about handling the "absent" case.

It looks better than the old-style way

Employee employee = employeeService.getEmployee();
if (employee != null) {
    ID id = employee.getId();
    if (id != null) {
        System.out.println(id);
    }
}

which makes it obvious that there may be else clauses.

The functional style

  • looks completely different (and is unreadable for people unused to it)
  • is a pain to debug
  • can't be easily extended for handling the "absent" case (orElse isn't always sufficient)
  • misleads to ignoring the "absent" case

In no case it should be used as a panacea. If it was universally applicable, then the semantics of the . operator should be replaced by the semantics of the ?. operator, the NPE forgotten and all problems ignored.


While being a big Java fan, I must say that the functional style is just a poor Java man's substitute of the statement

employeeService
.getEmployee()
?.getId()
?.apply(id => System.out.println(id));

well known from other languages. Do we agree that this statement

  • is not (really) functional?
  • is very similar to the old-style code, just much simpler?
  • is much more readable than the functional style?
  • is debugger-friendly?
  • You seem to be describing C#'s implementation of this concept with a language feature as entirely different to Java's implementation with a core library class. They look the same to me – Caleth Mar 5 '18 at 9:31
  • @Caleth No way. We could speak about the "concept of simplifying a null-safe evaluation", but I wouldn't call it a "concept". We can say that Java implements the same thing using a core library class, but it's just a mess. Just start with employeeService.getEmployee().getId().apply(id => System.out.println(id)); and make it null-safe once using the Safe navigation operator and once using the Optional. Sure, we could call it an "implementation detail", but implementation matters. There are cases when lamdas make code simpler and nicer, but here it's just an ugly abuse. – maaartinus Mar 5 '18 at 12:53
  • Library class: Optional.of(employeeService).map(EmployeeService::getEmployee).map(Employee::getId).ifPresent(System.out::println); Language feature: employeeService.getEmployee()?.getId()?.apply(id => System.out.println(id));. Two syntaxes, but they end up looking very similar to me. And the "concept" is Optional aka Nullable aka Maybe – Caleth Mar 5 '18 at 13:00
  • I don't understand why you think one of those is "ugly abuse", and the other good. I could understand you thinking both "ugly abuse" or both good. – Caleth Mar 5 '18 at 13:07
  • @Caleth One difference is that instead of adding two question marks, you have to rewrite it all. The problem is not that the Language feature and the Library class codes differ a lot. That's OK. The problem is that the original code and the Library class codes differ a lot. Same idea, different code. That's too bad. +++ What gets abused are lamdas for handling nullability. Lamdas are fine, but Optional is not. Nullability simply needs a proper language support like in Kotlin. Another problem: List<Optional<String>> is unrelated to List<String>, where we'd need them to be related. – maaartinus Mar 5 '18 at 13:22
0

Optional is a concept (a higher order type) coming from functional programming. Using it does away with nested null checking and saves you from the pyramid of doom.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.