On Fri, 10 Nov 2023 09:32:17 GMT, Viktor Klang <vkl...@openjdk.org> wrote:

>> This Pull-Request implements [JEP-461](https://openjdk.org/jeps/461)
>
> Viktor Klang has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Multiple improvements to Gatherer and Gatherers javadoc and restructuring 
> of Gatherers.java to put public at the top of the file.
>  - Augmenting Gatherer tests to include default implementation in Stream

I did another pass over the API/docs with the latest changes and it looks very 
good, this is a really nice API.

src/java.base/share/classes/java/util/stream/Gatherer.java line 91:

> 89:  *
> 90:  * <p>In addition to the predefined implementations in {@link Gatherers}, 
> the
> 91:  * static factory methods {@code of(...)} and {@code ofSequential(...)}

I assume both of these could be links.

src/java.base/share/classes/java/util/stream/Gatherer.java line 479:

> 477:          *
> 478:          * <p>If this method returns {@code false} then the next stage 
> does
> 479:          * not accept any more elements.

So once push returns false then any further attempts to push to this downstream 
will also return false.

src/java.base/share/classes/java/util/stream/Gatherer.java line 494:

> 492:          *          never return false again for the same instance.
> 493:          *
> 494:          * <p>By default this method returns {@code false}.

The text that specifies the default implementation can be in implSpec rather 
than the apiNote. I agree that false is the only sane default here.

src/java.base/share/classes/java/util/stream/Gatherer.java line 528:

> 526:          *         {@code false} if not
> 527:          */
> 528:         boolean integrate(A state, T element, Downstream<? super R> 
> downstream);

Integrator is silent on nulls and whether NPE is thrown.

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

PR Review: https://git.openjdk.org/jdk/pull/16420#pullrequestreview-1725141944
PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1389643747
PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1389620446
PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1389617514
PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1389625114

Reply via email to