On Mon, 30 Oct 2023 15:38:35 GMT, Viktor Klang <vkl...@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 44: > 42: * > 43: * <p>Examples of gathering operations include, but is not limited to: > 44: * grouping elements into batches, also known as windowing functions; Suggest for 1st sentence: "An intermediate operation that transforms a stream of input elements into a stream of output elements, optionally applying a final action when the end of the upstream is reached. The transformation may be stateless or stateful, and a Gatherer may buffer arbitrarily much input before producing any output." src/java.base/share/classes/java/util/stream/Gatherer.java line 49: > 47: * {@link java.util.stream.Gatherers} provides implementations of common > 48: * gathering operations. > 49: * incremental accumulation functions (prefix scan) src/java.base/share/classes/java/util/stream/Gatherer.java line 61: > 59: * > 60: * <p>Each invocation to {@link #initializer()}, {@link #integrator()}, > 61: * {@link #combiner()}, and {@link #finisher()} must return a semantically consistency: either all should be like "creation of a", or all should be an "ing" term (integrating, combining, performing)" 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. 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? 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? 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`. src/java.base/share/classes/java/util/stream/package-info.java line 636: > 634: * produces a {@code Map}, such as: > 635: * <pre>{@code > 636: * Map<Buyer, List<Transaction>> salesByBuyer Did you mean Gatherer.ofSequential? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1378997403 PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1379001492 PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1379003153 PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1379015289 PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1379028898 PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1378969982 PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1378989827 PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1378991117
