2017-11-05 16:06 GMT+01:00 Denis Kudriashov <[email protected]>: > > 2017-11-05 11:33 GMT+01:00 Nicolas Cellier <nicolas.cellier.aka.nice@ > gmail.com>: > >> >> Ah, I messed up, UndefinedSorter must not be chained, it must be a >> wrapper! >> Otherwise comparing to nil will resort to sorting by properties and >> fail... >> >> SortFunction>>undefinedFirst >> ^UndefinedSorter descending wrap: self >> >> UndefinedSorter>> collate: value1 with: value2 >> "sort all nil according to the direction (first if -1, last if >> +1), then" >> value1 ifNil: [value2 ifNil: [^0] ifNotNil: [^direction]]. >> value2 ifNil: [^direction negated]. >> ^sorterForNonNil collate: value1 with: value2 >> >> It's important to have the UndefinedSorter : >> - decoupled from property sort, because it can be generally usefull >> - collating 2 nil as 0, so that another property can be chained >> > > I like your idea. > It also forced me to think that direction itself should be implemented as > wrapper. I would name it InvertedSortFunction: > > InvertedSortFunction>>collate: value1 with: value2 > ^(actualSortFunction collate: value1 with: value2) * -1 > > > If we will do it then direction will be not part of SortFunction. And all > current functions will be in fact ascending. > And to explicitly reflect this fact I would introduce > AscendingSortFunction as their superclass. > InvertedSortFunction and ChainedSortFunction will stay subclasses of > SortFunction. > > So what you think? >
Yes, I was thinking the same. On another hand, direction makes thing symmetric and has its elegance too. What I don't like with it is that it forces the library to have two different messages for the same thing: - collate:with: in base class accounting for direction - threeWayCompare:with: in every subclass The fact to hardcode the direction in base class could be seen as an optimization too. I'm not sure what Sista optimization could bring in this case, because the selectors may get megamorphic... > >> >> In >> >> people sortBy: #name ascending undefinedFirst , #age descending >> >> we could then have people with name nil still sorted by age, what is not >> possible with current implementation >> >> >
