RFR: JDK-8319123 : Implementation of JEP-461: Stream Gatherers (Preview)

[Viktor Klang]

On Wed, 1 Nov 2023 16:16:38 GMT, Brian Goetz <briangoetz at openjdk.org> wrote:

>> This is a Draft PR for [JEP-461](https://openjdk.org/jeps/461)
>
> src/java.base/share/classes/java/util/stream/Gatherer.java line 137:
> 
>> 135:  *          })
>> 136:  *     );
>> 137:  * }
> 
> As a matter of style, I find it weird that `current` is assigned in both the constructor and in the lambda.  I'd be inclined to put all assignments in the State class (and expose more methods) or put all assignments in the lambdas (and ditch the ctor).  For an example this trivial, it doens't make so much differnce, but it gives a gentle push towards doing things more consistenly.

making `integrate` a method on State in this case is a bit of extra noise, which I wanted to avoid for something embedded in the Javadoc. I'll reduce some of the noise by assigning `current` in the member declaration instead of in an explicit ctor.

> src/java.base/share/classes/java/util/stream/Gatherers.java line 516:
> 
>> 514:      * evaluation can be out-of-sequence compared to the sequential encounter
>> 515:      * order of the stream.
>> 516:      *
> 
> How does this differ from Stream::peek?

@briangoetz Basically the same as `Stream::parallel().forEach` vs `Stream.parallel().forEachOrdered`.

> src/java.base/share/classes/java/util/stream/ReferencePipeline.java line 113:
> 
>> 111:     @Override
>> 112:     final StreamShape getOutputShape() {
>> 113:         return StreamShape.REFERENCE;
> 
> Not all parameters documented?

Good catch -- that was the same for [the other ctor](https://github.com/openjdk/jdk/pull/16420/files/6a7d16a1ffcf26a10ce443cf6bf1f5dd8695b808#diff-761882141e64725ba36ab6cb28dc7ea65deefa32f7b9810355de42126efefc6bR93) so it was carried over, I'll add params for both of the ctors:

> src/java.base/share/classes/java/util/stream/Stream.java line 1110:
> 
>> 1108:      * }</pre>
>> 1109:      *
>> 1110:      * <p>Like {@link #reduce(Object, BinaryOperator)}, {@code collect} operations
> 
> A brief description of what a Gatherer is here (around the time you say "extension point") would be helpful, something general like "Gatherers are highly flexible, expressing a many-to-many transformation on the elements of a stream, and support short-circuiting, blah blah blah".  ENough to make people want to read either the linked section or the Gatherer doc.  
> 
> The @impleNote is a little unclear; I'd use the format for other ops added after 8, such as takeWhile or mapMulti.
> 
> I'd add `@see Gatherer`.

@briangoetz 

>A brief description of what a Gatherer is here (around the time you say "extension point") would be helpful, something general like "Gatherers are highly flexible, expressing a many-to-many transformation on the elements of a stream, and support short-circuiting, blah blah blah". ENough to make people want to read either the linked section or the Gatherer doc.

Good point. I went with:

`Gatherers are highly flexible and can describe a vast array of possibly stateful operations, with support for short-circuiting, and parallelization.`

>The @impleNote is a little unclear; I'd use the format for other ops added after 8, such as takeWhile or mapMulti.

Do you mean the implSpec? Because the implNote just states: "Implementations of this interface should provide their own implementation of this method."

>I'd add @see Gatherer.

👍

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1379142852
PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1379143608
PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1379102144
PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1379120702