How to use Java 8 Optionals, performing an action if all three are present?

I have some (simplified) code that uses Java Optionals:

Optional<User> maybeTarget = userRepository.findById(id1);
Optional<String> maybeSourceName = userRepository.findById(id2).map(User::getName);
Optional<String> maybeEventName = eventRepository.findById(id3).map(Event::getName);

maybeTarget.ifPresent(target -> {
    maybeSourceName.ifPresent(sourceName -> {
        maybeEventName.ifPresent(eventName -> {
            sendInvite(target.getEmail(), String.format("Hi %s, $s has invited you to $s", target.getName(), sourceName, meetingName));
        }
    }
}

Needless to say, this looks and feels bad. But I can't think of another way to do this in a less-nested and more readable way. I considered streaming the 3 Optionals, but discarded the idea as doing a .filter(Optional::isPresent) then a .map(Optional::get) feels even worse.

So is there a better, more 'Java 8' or 'Optional-literate' way of dealing with this situation (essentially multiple Optionals all needed to compute a final operation)?

I think to stream the three Optionals is an overkill, why not the simple

if (maybeTarget.isPresent() && maybeSourceName.isPresent() && maybeEventName.isPresent()) {
  ...
}

In my eyes, this states the conditional logic more clearly compared to the use of the stream API.

Using a helper function, things at least become un-nested a little:

@FunctionalInterface
interface TriConsumer<T, U, S> {
    void accept(T t, U u, S s);
}

public static <T, U, S> void allOf(Optional<T> o1, Optional<U> o2, Optional<S> o3,
       TriConsumer<T, U, S> consumer) {
    o1.ifPresent(t -> o2.ifPresent(u -> o3.ifPresent(s -> consumer.accept(t, u, s))));
}

allOf(maybeTarget, maybeSourceName, maybeEventName,
    (target, sourceName, eventName) -> {
        /// ...
});

The obvious downside being that you'd need a separate helper function overload for every different number of Optionals

Since the original code is being executed for its side effects (sending an email), and not extracting or generating a value, the nested ifPresent calls seem appropriate. The original code doesn't seem too bad, and indeed it seems rather better than some of the answers that have been proposed. However, the statement lambdas and the local variables of type Optional do seem to add a fair amount of clutter.

First, I'll take the liberty of modifying the original code by wrapping it in a method, giving the parameters nice names, and making up some type names. I have no idea if the actual code is like this, but this shouldn't really be surprising to anyone.

// original version, slightly modified
void inviteById(UserId targetId, UserId sourceId, EventId eventId) {
    Optional<User> maybeTarget = userRepository.findById(targetId);
    Optional<String> maybeSourceName = userRepository.findById(sourceId).map(User::getName);
    Optional<String> maybeEventName = eventRepository.findById(eventId).map(Event::getName);

    maybeTarget.ifPresent(target -> {
        maybeSourceName.ifPresent(sourceName -> {
            maybeEventName.ifPresent(eventName -> {
                sendInvite(target.getEmail(), String.format("Hi %s, %s has invited you to %s",
                                                  target.getName(), sourceName, eventName));
            });
        });
    });
}

I played around with different refactorings, and I found that extracting the inner statement lambda into its own method makes the most sense to me. Given source and target users and an event -- no Optional stuff -- it sends mail about it. This is the computation that needs to be performed after all the optional stuff has been dealt with. I've also moved the data extraction (email, name) in here instead of mixing it with the Optional processing in the outer layer. Again, this makes sense to me: send mail from source to target about event.

void setupInvite(User target, User source, Event event) {
    sendInvite(target.getEmail(), String.format("Hi %s, %s has invited you to %s",
               target.getName(), source.getName(), event.getName()));
}

Now, let's deal with the optional stuff. As I said above, ifPresent is the way to go here, since we want to do something with side effects. It also provides a way to "extract" the value from an Optional and bind it to a name, but only within the context of a lambda expression. Since we want to do this for three different Optionals, nesting is called for. Nesting allows names from outer lambdas to be captured by inner lambdas. This lets us bind names to values extracted from the Optionals -- but only if they're present. This can't really be done with a linear chain, since some intermediate data structure like a tuple would be necessary to build up the partial results.

Finally, in the innermost lambda, we call the helper method defined above.

void inviteById(UserId targetId, UserId sourceID, EventId eventId) {
    userRepository.findById(targetId).ifPresent(
        target -> userRepository.findById(sourceID).ifPresent(
            source -> eventRepository.findById(eventId).ifPresent(
                event -> setupInvite(target, source, event))));
}

Note that I've inlined the Optionals instead of holding them in local variables. This reveals the nesting structure a bit better. It also provides for "short-circuiting" of the operation if one of the lookups doesn't find anything, since ifPresent simply does nothing on an empty Optional.

It's still a bit dense to my eye, though. I think the reason is that this code still depends on some external repositories on which to do the lookups. It's a bit uncomfortable to have this mixed together with the Optional processing. A possibility is simply to extract the lookups into their own methods findUser and findEvent. These are pretty obvious so I won't write them out. But if this were done, the result would be:

void inviteById(UserId targetId, UserId sourceID, EventId eventId) {
    findUser(targetId).ifPresent(
        target -> findUser(sourceID).ifPresent(
            source -> findEvent(eventId).ifPresent(
                event -> setupInvite(target, source, event))));
}

Fundamentally, this isn't that different from the original code. It's subjective, but I think I prefer this to the original code. It has the same, fairly simple structure, although nested instead of the typical linear chain of Optional processing. What's different is that the lookups are done conditionally within Optional processing, instead of being done up front, stored in local variables, and then doing only conditional extraction of Optional values. Also, I've separated out data manipulation (extraction of email and name, sending of message) into a separate method. This avoids mixing data manipulation with Optional processing, which I think tends to confuse things if we're dealing with multiple Optional instances.

How about something like this

 if(Stream.of(maybeTarget, maybeSourceName,  
                        maybeEventName).allMatch(Optional::isPresent))
  {
   sendinvite(....)// do get on all optionals.
  }

Having said that. If your logic to find in database is only to send mail, then if maybeTarget.ifPresent() is false, then there is no point to fetch the other two values, ain't it?. I am afraid, this kinda logic can be achieved only through traditional if else statements.

How to use Java 8 Optionals, performing an action if all three are present?

Related

Recent Posts