Exposing the CombineFn is necessary for use with composed combine or combining value state. There may be other reasons we made this visible, but these continue to justify it.
On Sun, May 14, 2017, 1:00 PM Reuven Lax <[email protected]> wrote: > I believe the reason why this is called Top.largest, is that originally it > was simply the comparator used by Top.largest - i.e. and implementation > detail. At some point it was made public and used by other transforms - > maybe making an implementation detail a public class was the real mistake? > > On Sun, May 14, 2017 at 11:45 AM, Davor Bonaci <[email protected]> wrote: > > > I agree this is an unfortunate name. > > > > Tangential: can we rename APIs now that the first stable release is > nearly > > done? > > Of course -- the "rename" can be done by introducing a new API, and > > deprecating, but not removing, the old one. Then, once we decide to move > to > > the next major release, the deprecated API can be removed. > > > > I think we should probably do the "rename" at some point, but I'd leave > the > > final call to the wider consensus. > > > > On Sat, May 13, 2017 at 5:16 PM, Wesley Tanaka <[email protected] > > > > wrote: > > > > > Using Top.Largest to sort a list of {2,1,3} produces {1,2,3}. This > > > matches the javadoc for the class, but seems counter-intuitive -- one > > might > > > expect that a Comparator called Largest would give largest items first. > > > I'm wondering if renaming the classes to Natural / Reversed would > better > > > match their behavior? > > > > > > --- > > > Wesley Tanaka > > > https://wtanaka.com/ > > >
