jOOQ / jOOL

jOOλ - The Missing Parts in Java 8 jOOλ improves the JDK libraries in areas where the Expert Group's focus was elsewhere. It adds tuple support, function support, and a lot of additional functionality around sequential Streams. The JDK 8's main efforts (default methods, lambdas, and the Stream API) were focused around maintaining backwards compatibility and implementing a functional API for parallelism.
http://www.jooq.org/products
Apache License 2.0
2.09k stars 168 forks source link

Added Seq.peekThrowable(Consumer<Throwable>) #315

Closed tlinkowski closed 7 years ago

tlinkowski commented 7 years ago

See #91 and #299.

@lukaseder, you should be aware that the implementation I provided here (which is the same as proposed in #299) does not have the exact semantics you expected. Namely, the following test passes:

// assert throws PeekedX EVEN when "peekThrowable" BEFORE "throw" but on the SAME Spliterator
assertThrows(PeekedX.class, () -> Seq.of(1, 2, 3)
    .peekThrowable(throwable -> {
        throw new PeekedX();
    })
    .peek(i -> {
        if (i == 2)
            throw new X();
    })
    .toList());

which means that peekThrowable works not only upstream but also downstream. The reason for this is (as far as I understand) that the action passed to the top-most (source) Spliterator.tryAdvance(Consumer<T> action) represents (in a way) all the cumulated operations (up to the terminal operation).

However, calling peekThrowable more up or down the stream can make a difference because we can catch two types of Throwables:

  1. Thrown inside action.apply(T) - these are caught both upstream and downstream
  2. Thrown in the remaining parts of Spliterator.tryAdvance() - these are caught only upstream
lukaseder commented 7 years ago

Thanks a lot for your PR!

does not have the exact semantics you expected

Maybe I didn't write it down this way, but the example is exactly what I would expect.

Thrown inside action.apply(T) - these are caught both upstream and downstream

Could you give an example for this?

tlinkowski commented 7 years ago

Well, in #299 you wrote:

Seq.seq(source)
   .filter(op1)
   .filter(op2)
   .peekThrowable(e -> ... /* only exceptions from op1 and op2 */)
   .filter(op3)
   .peekThrowable(e -> ... /* exceptions from op1, op2, or op3 */)
   .collect(collector);

but we really have:

Seq.seq(source)
   .filter(op1)
   .filter(op2)
   .peekThrowable(e -> ... /* all exceptions from op1 and op2 + most exceptions from op3 */)
   .filter(op3)
   .peekThrowable(e -> ... /* all exceptions from op1, op2, or op3 */)
   .collect(collector);

for example:

Seq.of(1, 2, 3)
   .filter(i -> i > 1)
   .filter(i -> i <= 3)
   .peekThrowable(System.out::println) // will print RuntimeException
   .filter(i -> {
     if (i == 3)
         throw new RuntimeException();
     return i < 3;
  })
  .toList();
lukaseder commented 7 years ago

Oh, interesting, thanks for clarifying. Then I don't agree with the status quo. Will revert. That's very surprising and doesn't work like peek()...