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/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]].

��������������