2017-11-05 0:37 GMT+01:00 Nicolas Cellier < [email protected]>:
> 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]].
