On Mon, 15 Nov 2021 18:08:22 GMT, Philippe Marschall <d...@openjdk.java.net> 
wrote:

> I have a similar project at 
> [empty-streams](https://github.com/marschall/empty-streams). A couple of 
> notes:
> 
> 1. I found the need for streams to be stateful. I had need for the following 
> state:
>    
>    1. closed
>    2. ordered
>    3. parallel
>    4. sorted

I found the need for ALL the Spliterator characteristics, plus some others that 
I interleaved between the Spliterator bits. That way I could fit them into a 
single int.

Parallel was too tricky though, so in that case I return a new stream built 
with the standard StreamSupport(spliterator, true).

>    5. closeHandler

Similarly when someone calls closeHandler(), I also return a new stream built 
with StreamSupport.

>    6. comparator (on EmptyStream)

Yes, we need this for TreeSet and ConcurrentSkipListSet. I was hoping to avoid 
it, but there is no way around if we want to be completely correct.

>       A shared instance can not be used because of `#close`.

Agreed. I fixed that.

> 2. I have a `PrimitiveIterator` that short circuits `#next` and 
> `#forEachRemaining` as well.

Oh that's a good suggestion - thanks.
> 3. I made many methods just return `this` after checking for operated on and 
> closed:
>    
>    1. `#filter` `#map`, `#flatMap`, `#mapMulti`, `#distinct`, `#peek`, 
> `#limit`, `#skip`, `#dropWhile`, `#takeWhile`.

I now always return a new EmptyStream. Fortunately escape analysis gets rid of 
a lot of objects.

>    2. These do a state change state as well `#sequential`, `#parallel`, 
> `#unordered`, `#sorted`, `#onClose`.

unordered() only does a state change if it currently ORDERED, otherwise it is 
correct to return this. The logic is convoluted and weird though. I have a more 
thorough test case that I need to incorporate into my EmptyStreamTest and which 
tests all the JDK collections, including the wrapper and immutable collections.

> 4. I made my `EmptyBaseStream` implement `BaseStream` and make 
> `EmptyIntLongDoubleStream` extend from this class as `IntLongDoubleStream` 
> all extend `BaseStream`. This allowed me to move the following methods up in 
> the hierarchy `#isParallel` , `#onClose`, `#sequential`, `#parallel`, 
> `#unordered`.

Interesting idea. I'm not sure if that would work for me. I will have a look if 
I can also do this, but I doubt it. A lot of the methods, especially the 
primitive ones, are repeats of each other. It might be really nice to put that 
into a common superclass. 

> 5. Is there any reason why you make `#parallel` "bail out" to `StreamSupport` 
> rather than just do a state change?

I tried to keep the characteristics identical to what we had before and 
parallel seemed particularly challenging to get right. I might attempt this 
again at a later stage, but for now I want to keep it like it is. I don't think 
parallel() is the most common use case, except with very large collections 
perhaps. Even though I thought I was careful with the characteristics, I still 
have a few edge cases I need to iron out.

-------------

PR: https://git.openjdk.java.net/jdk/pull/6275

Reply via email to