The use case is to wrap some return value in an Optional to communicate two scenarios:

1. the return value is not present intentionally
2. the return value should be present, but was null unexpectedly

For example:

Optional<Foo> myMethodToGetAFoo() {
  boolean shouldWeGetAFoo = [business logic];
  if (!shouldWeGetAFoo) {
    return Optional.empty(); // communicate that we intentionally don't return a foo
  }
  Foo myFooOrNull = serviceCallToGetAFoo();
  Optional<Foo> returnValue = Optional.ofNullable(myFooOrNull); // communicate that we do return a foo, which may be null
}

void myMethodThatUsesAFoo() {
  Optional<Foo> myFoo = myMethodToGetAFoo();
  // what I would like to do
  if (myFoo.isPresent()) {
    if (myFoo.get() == null) { // ******* however, this throws NoSuchElementException if it is of null
      // should have been present, but service call returned null (bad)
      log.error("bad"); return;
    }
    myFoo.get().useAsNormal();
  } else {
    // intentionally not present (fine)
    log.info("all good"); return;
  }
}

However, this is not possible because calling get() on an Optional of a null value throws a NoSuchElementException. This is problematic in this use case because no such element is overloaded to mean both:

1. The optional is empty
2. The optional is of a null value

Is there a way to distinguish between these scenarios? I understand this is precisely the intent of Optionals, to prevent doing get() and getting a null value, but I am wondering if there is some other pattern to achieve this.

CodePudding user response：

Here's an example of how you can quickly encapsulate that pattern into a utility class:

import java.util.function.Function;
import java.util.function.Supplier;

public class Result<T> {
    private final boolean required;
    private final T value;

    private Result(boolean required, T value) {
        this.required = required;
        this.value = value;
    }

    public static <T> Result<T> notRequired() {
        return new Result<>(false, null);
    }

    public static <T> Result<T> ofNullable(T value) {
        return new Result<>(true, value);
    }

    public <U> U consume(Supplier<U> notRequired, Supplier<U> unavailable, Function<T,U> available) {
        if (!required) {
            return notRequired.get();
        } else {
            return value == null ? unavailable.get() : available.apply(value);
        }
    }

    public static void main(String[] args) {
        Result<Integer> r = Result.ofNullable(10);
        System.out.println(r.consume(() -> "all good", () -> "bad", x -> "$"   x));
    }
}

There are many details you could change in this implementation:

• Use Optional instead of a nullable field
• A more fluent interface with three methods for the three cases instead of a single consume.
• A factory method which abstracts some of the details of how the three cases occur, to reduce boilerplate in myMethodToGetAFoo.

CodePudding user response：

So in short, there are two cases in your business logic, and you need to differentiate between them, which is not possible with the current implementation.

That's fine and that's understandable.

But the direction in which you expect to find a solution seems to be wrong.

The first thing to keep in mind, is that both null and an empty Optional are intended to convey the same thing - no data (hence, no reference to the object). It's a smell when one of these is getting a special meaning in your application. That's not the way to go. And inventing a new container would be the same thing in disguise.

To address the problem properly, it's crucial to understand that the current problem statement is incorrect.

Possible solutions

Firstly, you need to these two cases you want to discriminate between carefully. Are they both should be treated as normal?

I suspect that in the first case, when you deliberately don't want to return, the data might be an abnormal (like wrong credentials, invalid amount to withdraw, etc.). And the way you're logging this event log.error() imply that it's not how expect your app to function.

If this assumption is correct, then instead of returning, you can throw an exception.

Optional<Foo> myMethodToGetAFoo() {
    boolean shouldWeGetAFoo = [business logic];
    
    if (!shouldWeGetAFoo) {
        throw new MyException("You shall not pass!"); // use the appropriate exception type and message here
    }
    
    Foo myFooOrNull = serviceCallToGetAFoo();
    return Optional.ofNullable(myFooOrNull); // optional either empty or contains a result
}

And if you're using a framework, like Spring, you can leverage its built-in exception-handling mechanics to treat this exception properly.

If you don't want to resort to throwing an exception, which would be in contradiction with your intention of this behavior as abnormal via log.error(), an alternative could be to make the Foo implement the Null object pattern.

A "null-instance" of Foo (which can be stored in a static final field) can be provided to signify that the data was expected to be returned, but the appropriate information wasn't found.

Optional<Foo> myMethodToGetAFoo() {
    boolean shouldWeGetAFoo = [business logic];
    
    if (!shouldWeGetAFoo) {
        return Optional.empty(); // communicate that we intentionally don't return a foo
    }
    
    Foo myFooOrNull = serviceCallToGetAFoo();
    return Optional.ofNullable(null).or(() -> Optional.of(Foo.NULL_INSTANCE)); // communicate that we do return a foo, which may be null
}

And that how the logic of your second method would look in this case:

void myMethodThatUsesAFoo() {

    Optional<Foo> myFoo = myMethodToGetAFoo();

    myFoo.ifPresentOrElse(foo -> {
            if (foo == Foo.NULL_INSTANCE) log.error("bad");
            foo.useAsNormal(); // behavior of NULL_INSTANCE.useAsNormal() should be nutral
        },
        () -> { log.info("all good"); }
    );
}

Note:

• Every time you're using Optional.isPresent() in your code - pause for a second, most likely you're not leveraging Optional API to its full power. When you're accustomed to it, you rarely need to resort isPresent() checks.
• I would suggest the first approach over the second one. Since it is more explicit, easier to grasp and implement, and also consistent with your intention of logging that something serious has happened.