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?
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