sure, if you prefer, it's fine for me. Remi
On January 12, 2018 6:09:27 PM UTC, Paul Sandoz <paul.san...@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, fo...@univ-mlv.fr wrote: >> >> >> >> De: "Paul Sandoz" <paul.san...@oracle.com> >> À: "Remi Forax" <fo...@univ-mlv.fr> >> Cc: "core-libs-dev" <core-libs-dev@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 <fo...@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.
