trjhtr

Using Java 8, I'd like to sequence my eithers using a Collector:

Sequencing a stream of eithers means for me:

• If there's at least one left either inside the stream, I expect the Collector to return the first left either.


• If the eithers in the stream are exclusively right, I expect the Collector to return a list of each element inside the right eithers. That list is wrapped in a right either.

For example:

// Produces Either.Right([1, 2])
Stream.of(Either.right("1"), Either.right("2"))
 .collect(sequence());

// Produces Either.Left(too bad)
Stream.<Either<String, Integer>>of(Either.right(1), Either.left("too bad"),
 Either.left("no"))
 .collect(sequence());

I'm using Fugue's Either type.

I came up with a solution that works for me but, oh my, the type of that thing:

/**
 * Creates a @link Collector that produces the first left
 * @link Either either from a @link Stream
 * of @link Eithers or all the right elements if there is
 * no left element in the @link Stream.
 * <p>
 * Note: This is untested for parallel streams.
 *
 * @param <L> the type of left elements in the @link Eithers
 * @param <R> the type of right elements in the @link Eithers
 * @return a @link Collector sequencing a @link Stream of @link Eithers
 */
public static <L, R> Collector<Either<L, R>,
 List<Either<L, List<R>>>,
 Either<L, List<R>>> sequence() 

 Supplier<String> getErrorMsg =
 () -> "Couldn't get either although it is right";
 // The combiner combines partial results of the collector.
 // Collector demands that we use a mutable container for the either.
 // We chose a list for that.
 BinaryOperator<List<Either<L, List<R>>>> combiner =
 (firstEitherContainer, secondEitherContainer) -> 
 // When both eithers are right, we combine their lists
 if (firstEitherContainer.get(0).isRight() &&
 secondEitherContainer.get(0).isRight()) 
 firstEitherContainer.get(0).getOrError(getErrorMsg)
 .addAll(secondEitherContainer.get(0)
 .getOrError(getErrorMsg));
 return firstEitherContainer;
 // We found a left either -> Return that,
 // throwing away the accumulated right eithers
 else if (firstEitherContainer.get(0).isRight() &&
 secondEitherContainer.get(0).isLeft()) 
 return secondEitherContainer;
 else 
 // The first either was already left, continue with that
 return firstEitherContainer;
 
 ;
 // The accumulator merges the elements inside the stream.
 // Again, the Collector demands that we use a mutable container
 // for the either, which is our list.
 BiConsumer<List<Either<L, List<R>>>, Either<L, R>> accumulator =
 (acc, either) -> 
 // Both eithers are right -> Merge their lists
 if (acc.get(0).isRight() && either.isRight()) 
 acc.get(0).forEach(rights ->
 rights.add(either.getOrError(getErrorMsg)));
 
 // We found the first left either ->
 // This either is now the accumulator
 else if (acc.get(0).isRight() && either.isLeft()) 
 acc.set(0, Either.left(either.left().get()));
 
 // The accumulator already contained a left either ->
 // Leave the accumulator as it is
 ;
 // Get the either from the list
 Function<List<Either<L, List<R>>>, Either<L, List<R>>> finisher =
 eithers -> eithers.get(0);

 return Collector.of(
 // The supplier for the start element has to be a mutable container.
 // Also, the list inside the either must be mutable since we
 // add other values to it.
 () -> Arrays.asList(Either.right(new ArrayList<>())),
 accumulator,
 combiner,
 finisher);

I would love to get feedback on that sequence function, especially on how to achieve the same with less code and shorter type signatures.

I agree with the answer given by Stingy. Also note that with the collector implementation you are not short-circuiting, so with a stream of a billion elements, you will still process them all even when the first element encountered is a left.

That being said, I will comment on a thing or two for this implementation:

• 
  

  Users of your collector generally do not really care about the type of the accumulator, so you can wildcard the type instead:

  
  
  

  <L, R> Collector<Either<L, R>, ?, Either<L, List<R>>> sequence()
  

  
  


• 
  

  I would swap the checks in your combiner, it makes it more readable in my opinion:

  
  
  

  Either<L, List<R>> firstEither = firstEitherContainer.get(0);
  if (firstEither.isLeft()) 
   return firstEitherContainer;
  
  Either<L, List<R>> secondEither = secondEitherContainer.get(0);
  if (secondEither.isLeft()) 
   return secondEitherContainer;
  
  firstEither.getOrError(getErrorMsg).addAll(secondEither.getOrError(getErrorMsg));
  return firstEitherContainer;
  

  
  

You are horrified by "the type of that thing" because you are using stream operations for something they were not meant for. The purpose of streams is to perform pre-defined operations on an arbitrary number of elements of a kind. However, you have a scenario where the operation you want to perform depends on the nature of the stream itself. I did not really inspect your code in detail, because, based on your description of what you want to do, my suggestion would be to use a different approach altogether:

It is possible to obtain an iterator over the elements of a stream by calling iterator() on a stream (which is a terminal operation, so the stream will be consumed after creating the iterator, which means that you can obtain the iterator only once). Using that iterator, you can simply accumulate the elements inside the encountered Eithers in a List as long as they are all right eithers. If you encounter a left either, you stop iterating and return this left either. If the iterator is exhausted before you encounter a left either, you just wrap the now fully populated list in a right either and return that either.