Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

"Unwrap" Optional of Optional.empty before return in a Java stream (or Parallel Stream)

I'm having trouble with this simple thing. I have a method returning an Optional<TypeA>:

public Optional<TypeA> getFirst() {
  return set.stream().filter(it -> it.getStatus() == SUCCESS).findFirst(); // set is a Set<TypeA>
}

...then I'm calling it from:

public boolean hasFirst() {
  return types.parallelStream() // types is a List<TypeB> that has a Set<TypeA>
    .filter(it -> it.getTotalAmount() > 0)
    .map(TypeA::getFirst)
    .findFirst() // this can return an Optional[Optional.empty]
    .isPresent();
}

The issue is that, an Optional of an empty Optional is returning true, as expected, so I have no idea how to get this one right!

Any clues?

like image 505
x80486 Avatar asked Dec 06 '22 15:12

x80486


2 Answers

You could use Optional#flatMap to 'unwrap' the inner optional:

public boolean hasFirst() {
   return types.parallelStream()
       .filter(it -> it.getTotalAmount() > 0)
       .map(TypeA::getFirst)
       .findFirst() // return Optional[Optional.empty]
       .flatMap(i -> i) // <---
       .isPresent(); // true
}

If a value is present, apply the provided Optional-bearing mapping function to it, return that result, otherwise return an empty Optional.

like image 141
Jorn Vernee Avatar answered Jan 30 '23 05:01

Jorn Vernee


Using .findFirst().isPresent() is a common anti-pattern. If you only want to know whether there is a match, there is no need to ask for the first one, especially, if you intend to use a parallel Stream, where searching for the first match rather than any match can have significant performance drawbacks. So findAny would be preferrable, but it is not necessary to ask for a value at all, when you only want to know, whether there is a match:

public boolean hasFirst() {
  return types.parallelStream() // types is a List<TypeB> that has a Set<TypeA>
    .filter(it -> it.getTotalAmount() > 0)
    .anyMatch(obj -> obj.getFirst().isPresent());
}

It’s not clear, how your original code is supposed to work, if types is a list of TypeB instances and getFirst() is a method declared in TypeA, as TypeA::getFirst indicated, however, I think, you get the picture…

I recommend to also rename the method hasFirst() to hasAny(). Of course, a set having any matching element will also have a first one, but this focussing on “first” is misleading, especially as the TypeA elements are in a Set, which usually implies that there is no defined order at all.

like image 25
Holger Avatar answered Jan 30 '23 06:01

Holger