I have no idea what is a collation and the class comments does not really help. On Sun, Nov 5, 2017 at 12:56 AM, Nicolas Cellier <nicolas.cellier.aka.nice@gmail.com> wrote:
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
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/cf0b77a3d0babf9b09bcec9926e5a5fa... 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]].