Hi guys Do you have some nice comments somewhere? because I do not understand anything about this thread. I check the code and the comments are well... unclear and confusing.
Stef On Sun, Nov 5, 2017 at 4:15 PM, Nicolas Cellier <[email protected]> wrote: > > > 2017-11-05 16:06 GMT+01:00 Denis Kudriashov <[email protected]>: >> >> >> 2017-11-05 11:33 GMT+01:00 Nicolas Cellier >> <[email protected]>: >>> >>> >>> 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 >>> >> >
