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

IllegalStateException on Seq#append after Seq#forEach #365

Closed witoldsz closed 4 years ago

witoldsz commented 4 years ago

Steps to reproduce the problem:

@Test
public void test() {
    var x = Seq.empty();
    x.forEach(System.out::println);
    x.append(3); // <-- java.lang.IllegalStateException: stream has already been operated upon or closed
}

Versions:

billoneil commented 4 years ago

@witoldsz That is expected behavior of a stream once you hit a terminal operation such as forEach you have consumed the stream already.

Simple pure Java example

IntStream stream = IntStream.of();
stream.forEach(System.out::println);
stream.forEach(System.out::println); // java.lang.IllegalStateException: stream has already been operated upon or closed
witoldsz commented 4 years ago

Well, you are right. I was using Seq as immutable List, since Java introduced immutable lists, but did not bother to add operations for them :-/

Will work this out, though. Thanks!

witoldsz commented 4 years ago

Well, this is surprising:

    @Test
    public void test() {
        var x = Seq.empty();
        System.out.println(x);
        x.forEach(System.out::println);
        System.out.println(x);
        x.append(3); // <-- no exception here :O
    }
lukaseder commented 4 years ago

x.append(3); // <-- no exception here :O

Well, we don't guarantee this exception. Should we?

witoldsz commented 4 years ago

Well, we don't guarantee this exception. Should we?

Well, no need to :)

It was just unexpected for me, because my code was (kind of) OK until I've removed one System.out.println and it started to crash :O

Oh my, it is SO PAINFUL to be back in Java world again… (not for long, hopefully), but it's so much easier with jOOQ/jOOL!

amaembo commented 4 years ago

From the spec:

A stream implementation may throw IllegalStateException if it detects that the stream is being reused. However, since some stream operations may return their receiver rather than a new stream object, it may not be possible to detect reuse in all cases

lukaseder commented 4 years ago

Thanks, @amaembo. I was afraid I had new work, here! :)

lukaseder commented 4 years ago

@witoldsz. It may be painful, but it will be a joy discussing things with you again 😁

witoldsz commented 4 years ago

I HAVE TO admit that the Seq.empty() might become a surprise quite often, when it's used as a seed, identity or default value in many situations.

The trouble is, when using things like Collections.emptyList() (or it's newer cousin List.of()), Optional.empty(), etc… you can easily misuse Seq.empty() as an immutable seed value, but Seq.empty() produces a mutable beast even though most of the operations on it return new Seq. It IS LOGICAL, because it's a stream after all… and Stream.empty() can cause you similar troubles.

Wonderful world of "pure" OOP hidden complexity.