+1 for a different reason (also: now is the time to revisit names and other bits of the State API before it is too late :-)
Folks may not have this catalog in their head. The classes / methods are: ValueState / value(...) BagState / bag(...) SetState / set(...) MapState / map(...) AccumulatorCombiningState / combiningValue(...) I propose renaming this last one to CombiningState / combining(...) to match here and also to match CombineFn. I propose renaming what is currently CombiningState to something like GroupingState and probably just moving to runners-core (as a trivial wrapper, since it won't be able to remain a superclass) since it is not useful for users. Kenn On Thu, Mar 30, 2017 at 9:32 AM, Jean-Baptiste Onofré <[email protected]> wrote: > +0 > > as the StateSpec takes AccumularCombiningState, it's already "specify" > indirectly. > > Regards > JB > > > On 03/30/2017 04:46 PM, Etienne Chauchot wrote: > >> Hi all, >> >> Just a 10 cents comment, but maybe a rename of public method >> nevertheless... >> >> There are AccumulatorCombiningState and CombiningState interfaces, the >> first >> extends the second >> >> The factory method StateSpecs.combiningValue returns a >> StateSpec<Object, ** AccumulatorCombiningState<InputT, AccumT, OutputT> >> ** > >> >> The method name seems confusing for a user, who would search for a factory >> method returning a CombiningState (but there is none). >> >> Maybe StateSpecs.accumulatorCombiningValue would be a better name? >> >> The same goes for StateSpecs.*CombiningValue methods that all return >> AccumulatorCombiningState but maybe the name would be too long. >> >> WDYT? >> >> Etienne >> >> > -- > Jean-Baptiste Onofré > [email protected] > http://blog.nanthrax.net > Talend - http://www.talend.com >
