[10] RFR 8075939: Stream.flatMap() causes breaking of short-circuiting of terminal operations

[Remi Forax]

sure, if you prefer, it's fine for me.

Remi


On January 12, 2018 6:09:27 PM UTC, Paul Sandoz <paul.sandoz at oracle.com> wrote:
>Hi Remi,
>
>Catching up after the holidays. I would prefer to leave both things as
>they are.
>
>Thanks,
>Paul.
>
>> On 22 Dec 2017, at 03:15, forax at univ-mlv.fr wrote:
>> 
>> 
>> 
>> De: "Paul Sandoz" <paul.sandoz at oracle.com>
>> À: "Remi Forax" <forax at univ-mlv.fr>
>> Cc: "core-libs-dev" <core-libs-dev at openjdk.java.net>
>> Envoyé: Vendredi 22 Décembre 2017 01:38:37
>> Objet: Re: [10] RFR 8075939: Stream.flatMap() causes breaking of
>short-circuiting of terminal operations
>> 
>> 
>> On 21 Dec 2017, at 15:46, Remi Forax <forax at univ-mlv.fr> wrote:
>> 
>> Hi Paul,
>> three things:
>> - I think you should add a comment to explain why you have chosen to
>create a the field downstream* in the primitive implementations,
>>  I suppose it's to avoid to allocate a lambda proxy at each call. 
>> 
>> Yes, i included this comment:
>> // cache the consumer to avoid creation on every accepted element
>> 
>> - the fields in the inner classes cancellationRequested and
>downstream* should be private.
>> 
>> Does it make any difference for such inner classes?
>> 
>> Usually, a stronger encapsulation is better than a weaker one.  Being
>an anonymous class only means that you can not access the type, not
>that the fields are not accessible.
>> 
>> (I lean towards package private as the default.)
>> 
>> I tend to think that before, but with nestmate around the corner,
>private now really means accessible in the same java file, so i do
>think that private should be the new default in nested classes.
>> 
>> 
>> 
>> - if you use var, you should use a meaningful name, here, 's' can be
>replaced by 'spliterator', making the code more readable.
>> 
>> 
>> Don’t agree in this case, given the locality of use and given the
>method name on the RHS.
>> 
>> It means that when you want to know what a variable is you have to
>read its initialization part, it's a bit of a stretch compare to what
>Brian said about var, we can use var because the variable name is
>meaningful enough, so the type is redundant. I suppose it's not a big
>deal if you read the code sequentially but i tend to read the meat part
>of the code and extend above and below, so i was focus on the code that
>uses downstreamAsInt when i ask myself what 's' was.
>> 
>> 
>> Thanks,
>> Paul.
>> 
>> cheers,
>> Rémi
>> 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.