Hi Anthony,
It's a good point.
You're correct in saying that `Stream.Builder` is a superset of `Consumer`,
however `Stream.Builder`'s usual usage is in the creation of new streams. To
use it in this context brings with it the connotation that we are going to
create a new stream, something we don't want to suggest. In its current state,
we push directly downstream and don't create an intermediate stream (like in
`flatMap`, for example).
With regards to the build operation, I have to disagree. The build method is
central to the `Stream.Builder`'s use case. In your example, I think it's
obvious that close shouldn't be called unless you wish the Stream to be closed.
However, in the case with build the opposite is true: it's not obvious that a
call to build shouldn't be made when you want to finish building your stream.
Both operations are similar, but `Consumer` doesn't bring this baggage with it
and therefore seems more fitting.
Kind regards,
Patrick
> On 4 Jul 2020, at 17:23, Anthony Vanelverdinghe <d...@anthonyv.be> wrote:
>
> On 02/07/2020 16:38, Patrick Concannon wrote:
>> Hi Anthony,
> Hi Patrick
>> Thanks for your suggestion of using a Stream.Builder instead of a Consumer.
> Thanks for your response. Since Stream.Builder extends Consumer, I don't see
> it as "instead of": the proposed signature is a strict superset of the actual
> signature. For example, it would accept a BiConsumer<Object,
> Stream.Builder<String>>, whereas the current signature doesn't.
>> However, one of the goals for mapMulti is not to create an additional Stream.
> Would you mind to elaborate on this? Given that Stream.Builder extends
> Consumer, I don't see why using Stream.Builder would imply having to create
> any more Streams than when using Consumer.
>> Also, I fear that using a builder, but throwing an exception on a call to
>> build, is counter-intuitive and perhaps confusing to the user.
> Point taken, but I beg to differ: to me this is no different than a method
> `void foo(Consumer<InputStream> bar)` which specifies that the given Consumer
> must not invoke `close` on the InputStream.
> Moreover, with the build step being the sole difference with `flatMap`, and
> the elimination of intermediary Streams being a rather obvious optimization,
> I believe most users would find it intuitive not to invoke `build` themselves
> (or they would have read the Javadoc and know not to :)).
> Finally, using Stream.Builder makes it intuitive for me what the method does,
> even without considering its name.
>
> Given the above, I still hold that Stream.Builder is a better choice to use
> in the signature. Even more so, I believe there are several things that hint
> at the need for an additional abstraction: your point about the potential
> confusion, the analogy with Collector (as already mentioned by Paul w.r.t.
> the order of the BiConsumer's type arguments), and the natural similarity
> between e.g. BiConsumer<T, Stream.Builder<R>> and Function<T, Stream<R>>.
>
> Therefore I'd like to propose the following (complete declarations below):
>
> * introduce a new interface: Transformer<T, I, R> extends Function<T, R>
> * adapt Collector to extend it: Collector<T, A, R> extends
> Transformer<Stream<T>, A, R>
>
> So a Transformer is nothing more than a Function which passes via an
> intermediary I to map T to R according to a certain protocol.
>
> Then a method with the following signature could be added to Stream:
> <R> Stream<R> flatMap(Transformer<? super T, ? super Stream.Builder<R>, ?
> extends Stream<R>> mapper)
>
> Such a Transformer could readily be created from a given BiConsumer (see
> Tranformer::of below).
>
> Since Transformer is a subinterface of Function, there wouldn't be any
> ambiguities w.r.t. overload resolution, so we could just name the new method
> `flatMap` (I assume such ambiguities are the reason why this currently isn't
> the case, since Function and BiConsumer are unrelated?).
>
> Below is a self-contained "demo", containing a simple Demo class, the
> declaration of Transformer, and the adapted declaration of Collector.
>
> Thanks in advance for any feedback.
>
> Kind regards
> Anthony
>
>
>
> import java.util.ArrayList;
> import java.util.List;
> import java.util.Set;
> import java.util.function.BiConsumer;
> import java.util.function.BinaryOperator;
> import java.util.function.Function;
> import java.util.function.Supplier;
> import java.util.stream.Stream;
>
> public class Demo {
>
> public static void main(String[] args) {
> BiConsumer<String, Stream.Builder<String>> anagrams = (word, builder)
> -> {
> if(word.equals("listen")) {
> builder.accept("silent");
> builder.accept("enlist");
> }
> };
>
> Transformer.of(anagrams::accept).apply("listen").forEachOrdered(System.out::println);
> }
>
> }
>
> interface Transformer<T, I, R> extends Function<T, R> {
>
> Supplier<? extends I> supplier();
>
> BiConsumer<? super T, ? super I> transformer();
>
> Function<? super I, ? extends R> finisher();
>
> @Override
> default R apply(T t) {
> var intermediary = supplier().get();
> transformer().accept(t, intermediary);
> return finisher().apply(intermediary);
> }
>
> static <U, V> Transformer<U, Stream.Builder<V>, Stream<V>>
> of(BiConsumer<? super U, ? super Stream.Builder<V>> transformer) {
> return new Transformer<>() {
> @Override
> public Supplier<Stream.Builder<V>> supplier() {
> return Stream::builder;
> }
>
> @Override
> public BiConsumer<U, Stream.Builder<V>> transformer() {
> return transformer::accept;
> }
>
> @Override
> public Function<Stream.Builder<V>, Stream<V>> finisher() {
> return Stream.Builder::build;
> }
> };
> }
>
> }
>
> interface Collector<T, A, R> extends Transformer<Stream<T>, A, R> {
>
> @Override
> Supplier<A> supplier();
>
> BiConsumer<A, T> accumulator();
>
> BinaryOperator<A> combiner();
>
> @Override
> Function<A, R> finisher();
>
> Set<Characteristics> characteristics();
>
> enum Characteristics {
> CONCURRENT,
> UNORDERED,
> IDENTITY_FINISH
> }
>
> @Override
> default BiConsumer<Stream<T>, A> transformer() {
> return (stream, intermediary) -> {
> var accumulator = accumulator();
> stream.forEachOrdered(t -> accumulator.accept(intermediary, t));
> };
> }
>
> }
>
>>
>> Kind regards,
>>
>> Patrick
>>
>>> On 25 Jun 2020, at 18:12, Anthony Vanelverdinghe <d...@anthonyv.be>
>>> <mailto:d...@anthonyv.be> wrote:
>>>
>>> Hi
>>>
>>> Given the signature of flatMap is:
>>> <R> Stream<R> flatMap(Function<? super T, ? extends Stream<? extends R>>
>>> mapper)
>>>
>>> I'd like to propose the following signature for the new method:
>>> <R> Stream<R> builderMap(BiConsumer<? super T, ? super Stream.Builder<R>>
>>> mapper)
>>>
>>> This way both methods are named "...Map", and the name "builderMap" follows
>>> naturally from the argument's type.
>>> If the given mapper invokes Stream.Builder::build, an IllegalStateException
>>> should be thrown.
>>>
>>> Kind regards,
>>> Anthony
>>>
>>> On 25/06/2020 02:58, Paul Sandoz wrote:
>>>> Hi,
>>>>
>>>> We traded CPS style for reusing existing functional interfaces. Originally
>>>> the signature (my first choice) was as you indicate.
>>>>
>>>> By chance it just so happens that the current signature is the same shape
>>>> as that for the accumulating argument type of the three arg collect
>>>> terminal operation:
>>>>
>>>> Stream
>>>> <R> R collect(Supplier<R> supplier,
>>>> BiConsumer<R, ? super T> accumulator,
>>>> BiConsumer<R, R> combiner);
>>>>
>>>> IntStream
>>>> <R> R collect(Supplier<R> supplier,
>>>> ObjIntConsumer<R> accumulator,
>>>> BiConsumer<R, R> combiner);
>>>>
>>>> Same for the accumulator of a Collector too.
>>>>
>>>> However, I suspect you would argue these terminal accumulation cases are
>>>> different from the intermediate case, as we are not accumulating but
>>>> passing or accepting (loosely returning) zero or more elements that
>>>> replace the input element.
>>>>
>>>> It’s my hope that generic specialization will allow the primitive stream
>>>> types to fade into the background, along with the primitive functional
>>>> interfaces. In that respect the addition of three functional interfaces
>>>> for use on the primitive stream types is not so terrible.
>>>>
>>>>
>>>> Regarding the name, you should have seen the first one :-) it was terrible.
>>>>
>>>> Here’s my few brush strokes on the bike shed. I wonder what people think
>>>> of mapAccept. The specification talks about accepting elements, because
>>>> that is the operative method name on Consumer. So we can say "T is
>>>> replaced with the elements accepted by the Consumer<R>", or “ The
>>>> Consumer<R> accepts the elements that replace T"
>>>>
>>>> Paul.
>>>>
>>>>
>>>>
>>>>> On Jun 24, 2020, at 1:01 PM, John Rose <john.r.r...@oracle.com>
>>>>> <mailto:john.r.r...@oracle.com> wrote:
>>>>>
>>>>> I like this new API point a lot; it allows flexible, local, temporary
>>>>> control inversion in the context of one stream transform.
>>>>>
>>>>> What’s the performance model? It seems like the SpinedBuffer
>>>>> implementation makes a worst-case assumption about the number
>>>>> of pending values, that there will be many instead of zero or one.
>>>>>
>>>>> But I guess the pipeline stuff already works in terms of pushes, so
>>>>> the good news might be that this is really just a drill-down from the
>>>>> user API into the kinds of operations (push-flavored) that go on
>>>>> most of the time.
>>>>>
>>>>> OK, so I like the function but I have a beef with its bike shed
>>>>> color. First of all, this is a control-inversion (CPS) pattern,
>>>>> which is very powerful but also notoriously hard to read.
>>>>> I think that in Java APIs, at least in Stream APIs, code is
>>>>> easier to read if the logical data flow is from left to right.
>>>>>
>>>>> (This is a language-specific observation. Apart from varargs,
>>>>> Java method APIs read favorably when extra control arguments
>>>>> are added onto the end of the argument list. Also, the convention
>>>>> for generic functional interfaces is that the return value type
>>>>> goes to the right, e.g., R in Function<A,R>.)
>>>>>
>>>>> So the BiConsumer is backwards, because the logical return
>>>>> should be written, if not as a true return (which would appear
>>>>> at the end of type parameter lists), at the end of the incoming
>>>>> parameters (and in the last type parameter).
>>>>>
>>>>> I also think “multi” is needlessly “learned” sounding. A simple
>>>>> spatial preposition would work well: mapThrough, mapAcross, etc.
>>>>> I think I prefer mapAcross because the term “across” can be read
>>>>> adverbially: “we are mapping T across to Consumer<R>”.
>>>>>
>>>>> So:
>>>>>
>>>>> mapAcross(BiConsumer<? super T, Consumer<R>> mapper)
>>>>> mapAcrossToInt(BiConsumer<? super T, IntConsumer> mapper)
>>>>> mapAcross(IntObjConsumer<IntConsumer> mapper)
>>>>>
>>>>> This does require additional FI’s like IntObjConsumer, but
>>>>> I think that is a negligible cost. Making the control inversion
>>>>> *readable* is the high order bit here, not minimizing the number
>>>>> of trivial FIs.
>>>>>
>>>>> (I almost hear David Bowman, in his space suit, saying, “My API…
>>>>> It’s full of bikesheds!” There’s a meme for that.)
>>>>>
>>>>> — John
>>>>>
>>>>> On Jun 24, 2020, at 3:57 AM, Patrick Concannon
>>>>> <patrick.concan...@oracle.com> <mailto:patrick.concan...@oracle.com>
>>>>> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Could someone please review myself and Julia's RFE and CSR for
>>>>>> JDK-8238286 - 'Add new flatMap stream operation that is more amenable to
>>>>>> pushing’?
>>>>>>
>>>>>> This proposal is to add a new flatMap-like operation:
>>>>>>
>>>>>> `<R> Stream<R> mapMulti(BiConsumer<Consumer<R>, ? super T> mapper)`
>>>>>>
>>>>>> to the java.util.Stream class. This operation is more receptive to the
>>>>>> pushing or yielding of values than the current implementation that
>>>>>> internally assembles values (if any) into one or more streams. This
>>>>>> addition includes the primitive variations of the operation i.e.
>>>>>> mapMultiToInt, IntStream mapMulti, etc.
>>>>>>
>>>>>> issue: https://bugs.openjdk.java.net/browse/JDK-8238286
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8238286>
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8238286>
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8238286>
>>>>>> csr: https://bugs.openjdk.java.net/browse/JDK-8248166
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8248166>
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8248166>
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8248166>
>>>>>>
>>>>>> webrev:
>>>>>> http://cr.openjdk.java.net/~pconcannon/8238286/webrevs/webrev.00/
>>>>>> <http://cr.openjdk.java.net/~pconcannon/8238286/webrevs/webrev.00/>
>>>>>> <http://cr.openjdk.java.net/~pconcannon/8238286/webrevs/webrev.00/>
>>>>>> <http://cr.openjdk.java.net/~pconcannon/8238286/webrevs/webrev.00/>
>>>>>> specdiff:
>>>>>> http://cr.openjdk.java.net/~pconcannon/8238286/specdiff/specout.00/overview-summary.html
>>>>>>
>>>>>> <http://cr.openjdk.java.net/~pconcannon/8238286/specdiff/specout.00/overview-summary.html>
>>>>>>
>>>>>> <http://cr.openjdk.java.net/~pconcannon/8238286/specdiff/specout.00/overview-summary.html>
>>>>>>
>>>>>> <http://cr.openjdk.java.net/~pconcannon/8238286/specdiff/specout.00/overview-summary.html>
>>>>>>
>>>>>>
>>>>>> Kind regards,
>>>>>> Patrick & Julia
