[FIXED] Aggregate values and convert into single type within the same Java stream

Issue

I have a class with a collection of Seed elements. One of the method's return type of Seed is Optional<Pair<Boolean, String>>.

I'm trying to loop over all seeds, find if any boolean value is true and at the same time, create a set with all the String values. For instance, my input is in the form Optional<Pair<Boolean, String>>, the output should be Optional<Signal> where Signal is like:

class Signal {
   public boolean exposure;

   public Set<String> alarms;

   // constructor and getters (can add anything to this class, it's just a bag)
}

This is what I currently have that works:

// Seed::hadExposure yields Optional<Pair<Boolean, String>> where Pair have key/value or left/right
public Optional<Signal> withExposure() {
  if (seeds.stream().map(Seed::hadExposure).flatMap(Optional::stream).findAny().isEmpty()) {
    return Optional.empty();
  }
  final var exposure = seeds.stream()
      .map(Seed::hadExposure)
      .flatMap(Optional::stream)
      .anyMatch(Pair::getLeft);
  final var alarms = seeds.stream()
      .map(Seed::hadExposure)
      .flatMap(Optional::stream)
      .map(Pair::getRight)
      .filter(Objects::nonNull)
      .collect(Collectors.toSet());
  return Optional.of(new Signal(exposure, alarms));
}

Now I have time to make it better because Seed::hadExposure could become and expensive call, so I was trying to see if I could make all of this with only one pass. I've tried (some suggestions from previous questions) with reduce, using collectors (Collectors.collectingAndThen, Collectors.partitioningBy, etc.), but nothing so far.

Solution

It's possible to do this in a single stream() expression using map to convert the non-empty exposure to a Signal and then a reduce to combine the signals:

Signal signal = exposures.stream()
    .map(exposure ->
        new Signal(
            exposure.getLeft(),
            exposure.getRight() == null
                ? Collections.emptySet()
                : Collections.singleton(exposure.getRight())))
    .reduce(
        new Signal(false, new HashSet<>()),
        (leftSig, rightSig) -> {
            HashSet<String> alarms = new HashSet<>();
            alarms.addAll(leftSig.alarms);
            alarms.addAll(rightSig.alarms);
            return new Signal(
                leftSig.exposure || rightSig.exposure, alarms);
        });

However, if you have a lot of alarms it would be expensive because it creates a new Set and adds the new alarms to the accumulated alarms for each exposure in the input.

In a language that was designed from the ground-up to support functional programming, like Scala or Haskell, you'd have a Set data type that would let you efficiently create a new set that's identical to an existing set but with an added element, so there'd be no efficiency worries:

filteredSeeds.foldLeft((false, Set[String]())) { (result, exposure) => 
  (result._1 || exposure.getLeft, result._2 + exposure.getRight)
}

But Java doesn't come with anything like that out of the box.

You could create just a single Set for the result and mutate it in your stream's reduce expression, but some would regard that as poor style because you'd be mixing a functional paradigm (map/reduce over a stream) with a procedural one (mutating a set).

Personally, in Java, I'd just ditch the functional approach and use a for loop in this case. It'll be less code, more efficient, and IMO clearer.

If you have enough space to store an intermediate result, you could do something like:

List<Pair<Boolean, String>> exposures = 
    seeds.stream()
        .map(Seed::hadExposure)
        .flatMap(Optional::stream)
        .collect(Collectors.toList());

Then you'd only be calling the expensive Seed::hadExposure method once per item in the input list.

Answered By - gabeg
Answer Checked By - Timothy Miller (JavaFixing Admin)