java 8 optional to replace return null

Question

I am refactoring the code to Java 8 and I want to replace null checks with Optional.

public Employee findEmployeeById(String id) {
    List<Employee> empList = .. //some db query
    return (empList.isEmpty() ? null : empList.get(0));
}

Optional.ofNullable(empList.get(0)) won't work as when it will throw IndexOutofBoundException

Or should I ideally replace null with Optional.empty()?

Answer 1

As @Jesper already mentioned in the comments, you have to check whether the list is empty and then return an empty Optional.

public Optional<Employee> findEmployeeById(String id) {
    List<Employee> empList = .. //some db query
    return empList.isEmpty() ? Optional.empty() : Optional.of(empList.get(0));
}

An Optional is a wrapper around a potentially null value that allows you to avoid explicitly checking for null when you use it.

Have a look at the Optional documentation to see what functionality it provides.

For example you can get an employee's name or "unknown" if it's absent, without checking for null:

Optional<Employee> emp = findEmployeeById(id);
String name = emp.map(Employee::getName).orElse("unknown");

You may read this post about Uses for Optional to see if it makes sense for you to use Optional.

Answer 2

To me the natural solution would be to keep your ? : construct as in Floern’s answer. If, however, you want to get rid of that, there is also an elegant solution without it:

public Optional<Employee> findEmployeeById(String id) {
    List<Employee> empList = .. //some db query
    return empList.stream().findFirst();
}

This gives you what you want because findFirst() returns an Optional. If you don’t care which element you get — or you know there’s never more than one — you may alternatively use findAny(), it too returns an Optional.

Answer 3

Why don't you simply replace your method with:

public Optional<Employee> findEmployeeById(String id) {
    List<Employee> empList = .. //some db query
    return (empList.isEmpty() ? Optional.empty() : 
              Optional.ofNullable(empList.get(0)));
}

I suggest you wrap the empList.get(0) in a Optional.ofNullable in case it might still be null.

As far as why that is better: think about the caller of the method. Whomever is now calling your method has to think what to actually do when the result is empty.

Besides you are now forced into writing code like:

Optional<Employee> emp = findEmployeeById("12");

if (emp.isPresent()) {

} else {
    ....
}

You can also chain this to become more fluent like:

emp.orElseThrow(RuntimeException::new)

Or other Optional methods.

That is simply not the case when you return an Employee. You are not even thinking (usually) to check if the reference is null.

That makes your code less error-prone and easier to understand.