Hi, I think this is a very good addition to the Collectors API. Whereas somehow specific, it will come in very handy when needed.
But the name... well, honestly, teeingAndThen doesn't tell me anything from the perspective of an user of the API. What is 'teeing'? What does 'tee' actually mean? Or perhaps... Ah! They've finally added teeing to the API! Great! Now let's check what 'tee' actually means for these guys. I'm not sure, maybe it sounds a little bit too technical, or too implementation-specific to me. If I forget for a minute about the 'how' and focus on the 'what', I see that the operation either delegates to two collectors and then merges the results, or that it is aggregating two collectors. So, if it's not too late for new proposals, here are my 2 options: - delegateAndMerge (not gerund, but I think that delegatingAndMerging ends up beeing too long and too gerund-ish) - aggregating (I admit that I've though about these names once I read the comments of the operations/classes in the linked resources). Thanks, fps.- El lun., 20 ago. 2018 a las 10:49, Tagir Valeev (<amae...@gmail.com>) escribió: > Hello! > > A CSR is created: > https://bugs.openjdk.java.net/browse/JDK-8209685 > (this is my first CSR, hopefully I did it correctly) > > With best regards, > Tagir Valeev. > On Mon, Aug 20, 2018 at 2:06 PM Peter Levart <peter.lev...@gmail.com> > wrote: > > > > Hi Tagir, > > > > I think this looks very good. It just needs a CSR. Will you file it? > > > > Regards, Peter > > > > On 08/19/2018 11:24 AM, Tagir Valeev wrote: > > > > Hello, Brian! > > > > Of the three phases, teeing is the most important and least obvious, so > > I think something that includes that in the name is going to be > > helpful. Perhaps "teeingAndThen" is more evocative and not totally > > unwieldy. > > > > Ok, sounds acceptable to me. Renamed pairing to teeingAndThen. > > > > By the way looking into CollectorsTest.java I found some minor things to > > cleanup: > > 1. `.map(mapper::apply)` and `.flatMap(mapper::apply)` can be replaced > with > > simple `.map(mapper)` and `.flatMap(mapper)` respectively > > > > Does IntelliJ have an inspection for eliminating such locutions? > > > > Sure, that's how I found them. Well, I took the liberty to fix these two > things. > > > > 2. In many methods redundant `throws ReflectiveOperationException` is > > declared while exception is never thrown > > > > For test code where a significant fraction of test cases are going to > > throw something, we often do this, since its easier to just uniformly > > tag such methods rather than thinking about which test methods actually > > throw the exception and which don't. So I think this is harmless > > (though cleaning it up is harmless too.) > > > > I'm not thinking about this, because my IDE thinks for me :-) Ok, I'll > > leave them as is for now. > > > > You may want to optimize the EnumSet mechanics for the case where > > neither collector has interesting characteristics. > > > > Added a special case when reported characteristics for either of > > collectors are empty or IDENTITY_FINISH only. > > I think this should be a common case. > > > > The updated webrev is posted here (along with Peter suggestion to > > rename finisher to merger): > > http://cr.openjdk.java.net/~tvaleev/webrev/8205461/r3/ > > Also copyright year is updated > > > > With best regards, > > Tagir Valeev > > > > >
