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

[Paul Sandoz]

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
>