2017-11-05 0:56 GMT+01:00 Nicolas Cellier < [email protected]>:
> > > 2017-11-05 0:37 GMT+01:00 Nicolas Cellier <nicolas.cellier.aka.nice@ > gmail.com>: > >> Hi all, >> I started a discussion with Denis in a pull request >> https://github.com/pharo-project/pharo/pull/430 >> But since it's going beyond simple code review, it probably has a better >> place here. >> >> First, I want to say thanks for enriching this original work of Travis >> Griggs. >> http://objology.blogspot.fr/2010/11/tag-sortfunctions-redux.html >> http://objology.blogspot.fr/2010/11/tag-sortfunctions.html >> <http://objology.blogspot.fr/2010/11/tag-sortfunctions-redux.html> >> >> Of course, I'm never satisfied. >> So I don't really appreciate the rewrite of space ship operator <=> into >> a more heavy threeWayCompareTo: >> >> In my eyes, it's so obvious that <=> means < or = or > in the context of >> SortFunction >> And also that the order matches the signs >> < = > >> -1 0 1 >> IOW result will be -1 if receiver is <, 0 if equal, +1 if superior >> >> #threeWayCompareTo: does not tell as well. >> But maybe It's a question of taste and I don't have the same eyes as >> Pharo people? >> To me it's also a question of respect to the original author, don't >> change a good selector for the sake of changing. >> >> Apart this detail, I wanted to speak about SortByPropertyFunction. >> SortByProperty is super usefull for composing / chaining like this: >> >> population sortBy: #name ascending , #age descending. >> >> But currently, SortByPropertyFunction is allways using the default >> hardcoded collation <=> ... He he, it's also notice it's shorter to write ;) >> >> Imagine that my properties are neither String nor Magnitude, or they are, >> but I want a different collation policy, because in French $é must not be >> sorted too far from $e. >> >> It could be written quite simply with: >> >> c sortBy: #name collatedInFrench ascending , #age descending >> >> With current implementation, collatedInFrench would have to use a block >> (thru a CollatorBlockFunction which is a proxy to the block in a >> SortFunction disguise): >> >> Symbol>>collatedInFrench >> "interpret self as a property" >> ^[:a :b | FrenchCollator collate: (self value: a) with: (self >> value: b)] >> >> But IMO SortByPropertyFunction should better feed a reified >> CollatorFunction, or said differently a SortFunction, as Denis said. >> >> Symbol>>collatedInFrench >> ^FrenchCollator new onProperty: self >> >> SortFunction>>onProperty: aValuable >> ^SortByPropertyFunction sortProperty: aValuable with: self >> >> What do you think? >> >> Nicolas >> >> > Also, by reading https://github.com/pharo-project/pharo/blob/ > cf0b77a3d0babf9b09bcec9926e5a5faaffe7580/src/SortFunctions- > Core/SortByPropertyFunction.class.st > it's now obvious to me that we should simply use an UndefinedCollator > wrapper if ever we want to sort undefined properties first or last. > > We should better work with composition rather than complexifying each > class. > > SortFunction>>undefinedFirst > ^UndefinedSorter ascending , 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] ifNotNil: [^0]]. > > > 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 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
