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
