about SortFunctions

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
52 messages Options
123
Reply | Threaded
Open this post in threaded view
|

about SortFunctions

Nicolas Cellier
Hi all,
I started a discussion with Denis in a pull request
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
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

Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Nicolas Cellier


2017-11-05 0:37 GMT+01:00 Nicolas Cellier <[hidden email]>:
Hi all,
I started a discussion with Denis in a pull request
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
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


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

Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Stephane Ducasse-3
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
<[hidden email]> wrote:

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

Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Stephane Ducasse-3
In reply to this post by Nicolas Cellier
>
> 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
>

Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Stephane Ducasse-3
In reply to this post by Nicolas Cellier
Hi nicolas


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

(I should say that I do not like collator as a name. It does not mean
anything to me.)
Now in general I think that we are abusing blocks like in Glamour
(because we want compact syntax).
My rule of thumb to compare little class vs. block is
- do we have optional state?
- do we have many arguments (where we have to learn by heart the order)?



>     Symbol>>collatedInFrench
>         ^FrenchCollator new onProperty: self
>
>     SortFunction>>onProperty: aValuable
>         ^SortByPropertyFunction sortProperty: aValuable with: self
>
> What do you think?
>
> Nicolas
>

Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Nicolas Cellier


2017-11-05 9:52 GMT+01:00 Stephane Ducasse <[hidden email]>:
Hi nicolas


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

(I should say that I do not like collator as a name. It does not mean
anything to me.)

Then use Sorter.

Now in general I think that we are abusing blocks like in Glamour
(because we want compact syntax).
My rule of thumb to compare little class vs. block is
- do we have optional state?
- do we have many arguments (where we have to learn by heart the order)?


You forgot one rule:
- is the block replicated in many places?
 (that rule should include client libraries, not only kernel image)


>     Symbol>>collatedInFrench
>         ^FrenchCollator new onProperty: self
>
>     SortFunction>>onProperty: aValuable
>         ^SortByPropertyFunction sortProperty: aValuable with: self
>
> What do you think?
>
> Nicolas
>

Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Nicolas Cellier
In reply to this post by Nicolas Cellier


2017-11-05 0:56 GMT+01:00 Nicolas Cellier <[hidden email]>:


2017-11-05 0:37 GMT+01:00 Nicolas Cellier <[hidden email]>:
Hi all,
I started a discussion with Denis in a pull request
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
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


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

Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Denis Kudriashov
In reply to this post by Nicolas Cellier
Hi Nicolas.

We had discussion on operator <=> in original case where SortFunction was integrated https://pharo.fogbugz.com/f/cases/17728/Integrate-Travis-Griggs-TAG-SortFunctions.


2017-11-05 0:37 GMT+01:00 Nicolas Cellier <[hidden email]>:
Hi all,
I started a discussion with Denis in a pull request
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
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


Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Nicolas Cellier


2017-11-05 15:27 GMT+01:00 Denis Kudriashov <[hidden email]>:
Hi Nicolas.

We had discussion on operator <=> in original case where SortFunction was integrated https://pharo.fogbugz.com/f/cases/17728/Integrate-Travis-Griggs-TAG-SortFunctions.



Thanks for the link Denis.
I'm happy to see that Sven used exactly same arguments as me :)
Everything was said...

I still disagree with this decision.
It would be nice to ask for vox populi in the mailing list for such decision.
Even if vox populi may not lead to the wisest decision, it sound less arbitrary...
At least it can give greater chances for expressing different opinions and arguments.
Bug reports are way too much confidential (and this one is not even public!).

But that's a only a small detail.
We'd better look foreward, and discuss the composability features, notably the fact that
- property sort does allways use the default sort <=> and cannot be composed
- Undefined sort function is a general utility and should be implemented in own class

Nicolas

2017-11-05 0:37 GMT+01:00 Nicolas Cellier <[hidden email]>:
Hi all,
I started a discussion with Denis in a pull request
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
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



Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Denis Kudriashov
In reply to this post by Nicolas Cellier

2017-11-05 11:33 GMT+01:00 Nicolas Cellier <[hidden email]>:

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

I like your idea. 
It also forced me to think that direction itself should be implemented as wrapper. I would name it InvertedSortFunction:

InvertedSortFunction>>collate: value1 with: value2
    ^(actualSortFunction collate: value1 with: value2) * -1

If we will do it then direction will be not part of SortFunction. And all current functions will be in fact ascending.
And to explicitly reflect this fact I would introduce AscendingSortFunction as their superclass.
InvertedSortFunction and ChainedSortFunction will stay subclasses of SortFunction.

So what you think?
 

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


Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Nicolas Cellier


2017-11-05 16:06 GMT+01:00 Denis Kudriashov <[hidden email]>:

2017-11-05 11:33 GMT+01:00 Nicolas Cellier <[hidden email]>:

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

I like your idea. 
It also forced me to think that direction itself should be implemented as wrapper. I would name it InvertedSortFunction:

InvertedSortFunction>>collate: value1 with: value2
    ^(actualSortFunction collate: value1 with: value2) * -1

If we will do it then direction will be not part of SortFunction. And all current functions will be in fact ascending.
And to explicitly reflect this fact I would introduce AscendingSortFunction as their superclass.
InvertedSortFunction and ChainedSortFunction will stay subclasses of SortFunction.

So what you think?

Yes, I was thinking the same.
On another hand, direction makes thing symmetric and has its elegance too.
What I don't like with it is that it forces the library to have two different messages for the same thing:
- collate:with: in base class accounting for direction
- threeWayCompare:with: in every subclass

The fact to hardcode the direction in base class could be seen as an optimization too.
I'm not sure what Sista optimization could bring in this case, because the selectors may get megamorphic...
 

 

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



Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Stephane Ducasse-3
Hi guys

Do you have some nice comments somewhere? because I do not understand
anything about this thread.
I check the code and the comments are well... unclear and confusing.

Stef

On Sun, Nov 5, 2017 at 4:15 PM, Nicolas Cellier
<[hidden email]> wrote:

>
>
> 2017-11-05 16:06 GMT+01:00 Denis Kudriashov <[hidden email]>:
>>
>>
>> 2017-11-05 11:33 GMT+01:00 Nicolas Cellier
>> <[hidden email]>:
>>>
>>>
>>> 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
>>
>>
>> I like your idea.
>> It also forced me to think that direction itself should be implemented as
>> wrapper. I would name it InvertedSortFunction:
>>
>> InvertedSortFunction>>collate: value1 with: value2
>>     ^(actualSortFunction collate: value1 with: value2) * -1
>>
>>
>> If we will do it then direction will be not part of SortFunction. And all
>> current functions will be in fact ascending.
>> And to explicitly reflect this fact I would introduce
>> AscendingSortFunction as their superclass.
>> InvertedSortFunction and ChainedSortFunction will stay subclasses of
>> SortFunction.
>>
>> So what you think?
>
>
> Yes, I was thinking the same.
> On another hand, direction makes thing symmetric and has its elegance too.
> What I don't like with it is that it forces the library to have two
> different messages for the same thing:
> - collate:with: in base class accounting for direction
> - threeWayCompare:with: in every subclass
>
> The fact to hardcode the direction in base class could be seen as an
> optimization too.
> I'm not sure what Sista optimization could bring in this case, because the
> selectors may get megamorphic...
>
>
>>
>>>
>>>
>>> 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
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Nicolas Cellier
I've put some of the proposed composition features into

http://source.squeak.org/inbox/Collections-nice.766.mcz
http://source.squeak.org/inbox/CollectionsTests-nice.283.mcz

SInce it's Squeak based, it uses <=>, but never mind, that's the same functions.


Stephane: the original comments were from Travis Griggs.
It's more a tutorial for using the SortFunction than a detailed explanation of implementation.

The original idea is that oSortFunction are composable.
For example, it's possible to chain sorting on a first criterion, then resort on a second criterion if objects have same rank with first.

For this, we need a to distinguish when objects have same rank (=) and cannot use a binary result (Boolean) like legacy sortBlock,
So we rather need a ternary comparison (<=>).

This thread is about extending composition, and generalizing implementation by composition (like Xtreams)
- for sorting nil first (or last)
- for reversing the order
- for sorting properties with any collation order, and not just default <=>

2017-11-05 17:52 GMT+01:00 Stephane Ducasse <[hidden email]>:
Hi guys

Do you have some nice comments somewhere? because I do not understand
anything about this thread.
I check the code and the comments are well... unclear and confusing.

Stef

On Sun, Nov 5, 2017 at 4:15 PM, Nicolas Cellier
<[hidden email]> wrote:
>
>
> 2017-11-05 16:06 GMT+01:00 Denis Kudriashov <[hidden email]>:
>>
>>
>> 2017-11-05 11:33 GMT+01:00 Nicolas Cellier
>> <[hidden email]>:
>>>
>>>
>>> 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
>>
>>
>> I like your idea.
>> It also forced me to think that direction itself should be implemented as
>> wrapper. I would name it InvertedSortFunction:
>>
>> InvertedSortFunction>>collate: value1 with: value2
>>     ^(actualSortFunction collate: value1 with: value2) * -1
>>
>>
>> If we will do it then direction will be not part of SortFunction. And all
>> current functions will be in fact ascending.
>> And to explicitly reflect this fact I would introduce
>> AscendingSortFunction as their superclass.
>> InvertedSortFunction and ChainedSortFunction will stay subclasses of
>> SortFunction.
>>
>> So what you think?
>
>
> Yes, I was thinking the same.
> On another hand, direction makes thing symmetric and has its elegance too.
> What I don't like with it is that it forces the library to have two
> different messages for the same thing:
> - collate:with: in base class accounting for direction
> - threeWayCompare:with: in every subclass
>
> The fact to hardcode the direction in base class could be seen as an
> optimization too.
> I'm not sure what Sista optimization could bring in this case, because the
> selectors may get megamorphic...
>
>
>>
>>>
>>>
>>> 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
>>>
>>
>


Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Denis Kudriashov
It is difficult to compare to current Pharo version.
Why you not produce Pharo pull request?

2017-11-06 0:06 GMT+01:00 Nicolas Cellier <[hidden email]>:
I've put some of the proposed composition features into

http://source.squeak.org/inbox/Collections-nice.766.mcz
http://source.squeak.org/inbox/CollectionsTests-nice.283.mcz

SInce it's Squeak based, it uses <=>, but never mind, that's the same functions.


Stephane: the original comments were from Travis Griggs.
It's more a tutorial for using the SortFunction than a detailed explanation of implementation.

The original idea is that oSortFunction are composable.
For example, it's possible to chain sorting on a first criterion, then resort on a second criterion if objects have same rank with first.

For this, we need a to distinguish when objects have same rank (=) and cannot use a binary result (Boolean) like legacy sortBlock,
So we rather need a ternary comparison (<=>).

This thread is about extending composition, and generalizing implementation by composition (like Xtreams)
- for sorting nil first (or last)
- for reversing the order
- for sorting properties with any collation order, and not just default <=>

2017-11-05 17:52 GMT+01:00 Stephane Ducasse <[hidden email]>:
Hi guys

Do you have some nice comments somewhere? because I do not understand
anything about this thread.
I check the code and the comments are well... unclear and confusing.

Stef

On Sun, Nov 5, 2017 at 4:15 PM, Nicolas Cellier
<[hidden email]> wrote:
>
>
> 2017-11-05 16:06 GMT+01:00 Denis Kudriashov <[hidden email]>:
>>
>>
>> 2017-11-05 11:33 GMT+01:00 Nicolas Cellier
>> <[hidden email]>:
>>>
>>>
>>> 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
>>
>>
>> I like your idea.
>> It also forced me to think that direction itself should be implemented as
>> wrapper. I would name it InvertedSortFunction:
>>
>> InvertedSortFunction>>collate: value1 with: value2
>>     ^(actualSortFunction collate: value1 with: value2) * -1
>>
>>
>> If we will do it then direction will be not part of SortFunction. And all
>> current functions will be in fact ascending.
>> And to explicitly reflect this fact I would introduce
>> AscendingSortFunction as their superclass.
>> InvertedSortFunction and ChainedSortFunction will stay subclasses of
>> SortFunction.
>>
>> So what you think?
>
>
> Yes, I was thinking the same.
> On another hand, direction makes thing symmetric and has its elegance too.
> What I don't like with it is that it forces the library to have two
> different messages for the same thing:
> - collate:with: in base class accounting for direction
> - threeWayCompare:with: in every subclass
>
> The fact to hardcode the direction in base class could be seen as an
> optimization too.
> I'm not sure what Sista optimization could bring in this case, because the
> selectors may get megamorphic...
>
>
>>
>>>
>>>
>>> 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
>>>
>>
>



Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Nicolas Cellier
Because it's a cross dialect library and because contributing in Squeak is so much easier for me.

I like the social power provided by github and i know how to use git, at least as an expert beginner.
But it took me two hours to pick a pharo image, fork and clone the pharo repository, pick a pharo VM (I ended up building my own),
retrieve my ssh pass phrase to avoid using github thru https,
search documentation of where should I put the image wrt to my cloned git repository,
search how to use iceberg,
and finally realized that the author/date would not even be preserved if we want to port this cross dialect library.
The dark theme upsets me, especially the green on blue, I could have searched how to change it, but there I stopped.

While doing all these things, I did not spent a minute focusing on the SortFunction subject.
I can see that as an investment, maybe I should have done it before, but it was too much for a Sunday evening.

2017-11-06 10:13 GMT+01:00 Denis Kudriashov <[hidden email]>:
It is difficult to compare to current Pharo version.
Why you not produce Pharo pull request?

2017-11-06 0:06 GMT+01:00 Nicolas Cellier <[hidden email]>:
I've put some of the proposed composition features into

http://source.squeak.org/inbox/Collections-nice.766.mcz
http://source.squeak.org/inbox/CollectionsTests-nice.283.mcz

SInce it's Squeak based, it uses <=>, but never mind, that's the same functions.


Stephane: the original comments were from Travis Griggs.
It's more a tutorial for using the SortFunction than a detailed explanation of implementation.

The original idea is that oSortFunction are composable.
For example, it's possible to chain sorting on a first criterion, then resort on a second criterion if objects have same rank with first.

For this, we need a to distinguish when objects have same rank (=) and cannot use a binary result (Boolean) like legacy sortBlock,
So we rather need a ternary comparison (<=>).

This thread is about extending composition, and generalizing implementation by composition (like Xtreams)
- for sorting nil first (or last)
- for reversing the order
- for sorting properties with any collation order, and not just default <=>

2017-11-05 17:52 GMT+01:00 Stephane Ducasse <[hidden email]>:
Hi guys

Do you have some nice comments somewhere? because I do not understand
anything about this thread.
I check the code and the comments are well... unclear and confusing.

Stef

On Sun, Nov 5, 2017 at 4:15 PM, Nicolas Cellier
<[hidden email]> wrote:
>
>
> 2017-11-05 16:06 GMT+01:00 Denis Kudriashov <[hidden email]>:
>>
>>
>> 2017-11-05 11:33 GMT+01:00 Nicolas Cellier
>> <[hidden email]>:
>>>
>>>
>>> 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
>>
>>
>> I like your idea.
>> It also forced me to think that direction itself should be implemented as
>> wrapper. I would name it InvertedSortFunction:
>>
>> InvertedSortFunction>>collate: value1 with: value2
>>     ^(actualSortFunction collate: value1 with: value2) * -1
>>
>>
>> If we will do it then direction will be not part of SortFunction. And all
>> current functions will be in fact ascending.
>> And to explicitly reflect this fact I would introduce
>> AscendingSortFunction as their superclass.
>> InvertedSortFunction and ChainedSortFunction will stay subclasses of
>> SortFunction.
>>
>> So what you think?
>
>
> Yes, I was thinking the same.
> On another hand, direction makes thing symmetric and has its elegance too.
> What I don't like with it is that it forces the library to have two
> different messages for the same thing:
> - collate:with: in base class accounting for direction
> - threeWayCompare:with: in every subclass
>
> The fact to hardcode the direction in base class could be seen as an
> optimization too.
> I'm not sure what Sista optimization could bring in this case, because the
> selectors may get megamorphic...
>
>
>>
>>>
>>>
>>> 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
>>>
>>
>




Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Denis Kudriashov
Oh, I hope we will be able improve this process.
But personally when I set up it first time everything start to be smooth. And when I repeat it from scratch it takes just couple of minutes.

2017-11-06 10:37 GMT+01:00 Nicolas Cellier <[hidden email]>:
Because it's a cross dialect library and because contributing in Squeak is so much easier for me.

I like the social power provided by github and i know how to use git, at least as an expert beginner.
But it took me two hours to pick a pharo image, fork and clone the pharo repository, pick a pharo VM (I ended up building my own),
retrieve my ssh pass phrase to avoid using github thru https,
search documentation of where should I put the image wrt to my cloned git repository,
search how to use iceberg,
and finally realized that the author/date would not even be preserved if we want to port this cross dialect library.
The dark theme upsets me, especially the green on blue, I could have searched how to change it, but there I stopped.

While doing all these things, I did not spent a minute focusing on the SortFunction subject.
I can see that as an investment, maybe I should have done it before, but it was too much for a Sunday evening.

2017-11-06 10:13 GMT+01:00 Denis Kudriashov <[hidden email]>:
It is difficult to compare to current Pharo version.
Why you not produce Pharo pull request?

2017-11-06 0:06 GMT+01:00 Nicolas Cellier <[hidden email]>:
I've put some of the proposed composition features into

http://source.squeak.org/inbox/Collections-nice.766.mcz
http://source.squeak.org/inbox/CollectionsTests-nice.283.mcz

SInce it's Squeak based, it uses <=>, but never mind, that's the same functions.


Stephane: the original comments were from Travis Griggs.
It's more a tutorial for using the SortFunction than a detailed explanation of implementation.

The original idea is that oSortFunction are composable.
For example, it's possible to chain sorting on a first criterion, then resort on a second criterion if objects have same rank with first.

For this, we need a to distinguish when objects have same rank (=) and cannot use a binary result (Boolean) like legacy sortBlock,
So we rather need a ternary comparison (<=>).

This thread is about extending composition, and generalizing implementation by composition (like Xtreams)
- for sorting nil first (or last)
- for reversing the order
- for sorting properties with any collation order, and not just default <=>

2017-11-05 17:52 GMT+01:00 Stephane Ducasse <[hidden email]>:
Hi guys

Do you have some nice comments somewhere? because I do not understand
anything about this thread.
I check the code and the comments are well... unclear and confusing.

Stef

On Sun, Nov 5, 2017 at 4:15 PM, Nicolas Cellier
<[hidden email]> wrote:
>
>
> 2017-11-05 16:06 GMT+01:00 Denis Kudriashov <[hidden email]>:
>>
>>
>> 2017-11-05 11:33 GMT+01:00 Nicolas Cellier
>> <[hidden email]>:
>>>
>>>
>>> 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
>>
>>
>> I like your idea.
>> It also forced me to think that direction itself should be implemented as
>> wrapper. I would name it InvertedSortFunction:
>>
>> InvertedSortFunction>>collate: value1 with: value2
>>     ^(actualSortFunction collate: value1 with: value2) * -1
>>
>>
>> If we will do it then direction will be not part of SortFunction. And all
>> current functions will be in fact ascending.
>> And to explicitly reflect this fact I would introduce
>> AscendingSortFunction as their superclass.
>> InvertedSortFunction and ChainedSortFunction will stay subclasses of
>> SortFunction.
>>
>> So what you think?
>
>
> Yes, I was thinking the same.
> On another hand, direction makes thing symmetric and has its elegance too.
> What I don't like with it is that it forces the library to have two
> different messages for the same thing:
> - collate:with: in base class accounting for direction
> - threeWayCompare:with: in every subclass
>
> The fact to hardcode the direction in base class could be seen as an
> optimization too.
> I'm not sure what Sista optimization could bring in this case, because the
> selectors may get megamorphic...
>
>
>>
>>>
>>>
>>> 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
>>>
>>
>





Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Sven Van Caekenberghe-2
In reply to this post by Nicolas Cellier


> On 6 Nov 2017, at 10:37, Nicolas Cellier <[hidden email]> wrote:
>
> Because it's a cross dialect library and because contributing in Squeak is so much easier for me.
>
> I like the social power provided by github and i know how to use git, at least as an expert beginner.
> But it took me two hours to pick a pharo image, fork and clone the pharo repository, pick a pharo VM (I ended up building my own),
> retrieve my ssh pass phrase to avoid using github thru https,
> search documentation of where should I put the image wrt to my cloned git repository,
> search how to use iceberg,
> and finally realized that the author/date would not even be preserved if we want to port this cross dialect library.

You are absolutely right, but it is improving and will get better.

> The dark theme upsets me, especially the green on blue, I could have searched how to change it, but there I stopped.

Then switch to the light/white theme, just one setting.

> While doing all these things, I did not spent a minute focusing on the SortFunction subject.
> I can see that as an investment, maybe I should have done it before, but it was too much for a Sunday evening.

I think what Denis means is, it would have been helpful to apply the change in Pharo. How you communicate that is less relevant, just saving the MC version and mailing it would already have been very helpful.

IMHO.

> 2017-11-06 10:13 GMT+01:00 Denis Kudriashov <[hidden email]>:
> It is difficult to compare to current Pharo version.
> Why you not produce Pharo pull request?
>
> 2017-11-06 0:06 GMT+01:00 Nicolas Cellier <[hidden email]>:
> I've put some of the proposed composition features into
>
> http://source.squeak.org/inbox/Collections-nice.766.mcz
> http://source.squeak.org/inbox/CollectionsTests-nice.283.mcz
>
> http://source.squeak.org/inbox/Collections-nice.766.diff
> http://source.squeak.org/inbox/CollectionsTests-nice.283.diff
>
> SInce it's Squeak based, it uses <=>, but never mind, that's the same functions.
>
>
> Stephane: the original comments were from Travis Griggs.
> It's more a tutorial for using the SortFunction than a detailed explanation of implementation.
>
> The original idea is that oSortFunction are composable.
> For example, it's possible to chain sorting on a first criterion, then resort on a second criterion if objects have same rank with first.
>
> For this, we need a to distinguish when objects have same rank (=) and cannot use a binary result (Boolean) like legacy sortBlock,
> So we rather need a ternary comparison (<=>).
>
> This thread is about extending composition, and generalizing implementation by composition (like Xtreams)
> - for sorting nil first (or last)
> - for reversing the order
> - for sorting properties with any collation order, and not just default <=>
>
> 2017-11-05 17:52 GMT+01:00 Stephane Ducasse <[hidden email]>:
> Hi guys
>
> Do you have some nice comments somewhere? because I do not understand
> anything about this thread.
> I check the code and the comments are well... unclear and confusing.
>
> Stef
>
> On Sun, Nov 5, 2017 at 4:15 PM, Nicolas Cellier
> <[hidden email]> wrote:
> >
> >
> > 2017-11-05 16:06 GMT+01:00 Denis Kudriashov <[hidden email]>:
> >>
> >>
> >> 2017-11-05 11:33 GMT+01:00 Nicolas Cellier
> >> <[hidden email]>:
> >>>
> >>>
> >>> 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
> >>
> >>
> >> I like your idea.
> >> It also forced me to think that direction itself should be implemented as
> >> wrapper. I would name it InvertedSortFunction:
> >>
> >> InvertedSortFunction>>collate: value1 with: value2
> >>     ^(actualSortFunction collate: value1 with: value2) * -1
> >>
> >>
> >> If we will do it then direction will be not part of SortFunction. And all
> >> current functions will be in fact ascending.
> >> And to explicitly reflect this fact I would introduce
> >> AscendingSortFunction as their superclass.
> >> InvertedSortFunction and ChainedSortFunction will stay subclasses of
> >> SortFunction.
> >>
> >> So what you think?
> >
> >
> > Yes, I was thinking the same.
> > On another hand, direction makes thing symmetric and has its elegance too.
> > What I don't like with it is that it forces the library to have two
> > different messages for the same thing:
> > - collate:with: in base class accounting for direction
> > - threeWayCompare:with: in every subclass
> >
> > The fact to hardcode the direction in base class could be seen as an
> > optimization too.
> > I'm not sure what Sista optimization could bring in this case, because the
> > selectors may get megamorphic...
> >
> >
> >>
> >>>
> >>>
> >>> 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
> >>>
> >>
> >
>
>
>
>


Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Nicolas Cellier


2017-11-06 10:51 GMT+01:00 Sven Van Caekenberghe <[hidden email]>:


> On 6 Nov 2017, at 10:37, Nicolas Cellier <[hidden email]> wrote:
>
> Because it's a cross dialect library and because contributing in Squeak is so much easier for me.
>
> I like the social power provided by github and i know how to use git, at least as an expert beginner.
> But it took me two hours to pick a pharo image, fork and clone the pharo repository, pick a pharo VM (I ended up building my own),
> retrieve my ssh pass phrase to avoid using github thru https,
> search documentation of where should I put the image wrt to my cloned git repository,
> search how to use iceberg,
> and finally realized that the author/date would not even be preserved if we want to port this cross dialect library.

You are absolutely right, but it is improving and will get better.


Nope, i don't buy the argument...
The information has simply been erased and won't reappear by operation of wishfull thinking
Even if we later add the blame API, the information of original authors is definitely lost.

That was already the case at each Pharo release, the mc ancestry was reset and it was very difficult to trace back.
7.0 is in continuity, just with a more radical eradication.

For Pharo, such compatibility is a drag, for me it's usefull, it's just that we have different trade offs
It's not for complaining, but trade offs are to be made explicit and assumed.


> The dark theme upsets me, especially the green on blue, I could have searched how to change it, but there I stopped.

Then switch to the light/white theme, just one setting.

> While doing all these things, I did not spent a minute focusing on the SortFunction subject.
> I can see that as an investment, maybe I should have done it before, but it was too much for a Sunday evening.

I think what Denis means is, it would have been helpful to apply the change in Pharo. How you communicate that is less relevant, just saving the MC version and mailing it would already have been very helpful.

IMHO.

Right, I can understand that cross dialect is becoming inconvenient...
That's why I gave the .diff URL just for giving a chance to glimpse at refactoring POC.

Pharo still has Monticello, so it could have worked (not sure how well without common ancestor)
The extra difficulty here is that package delimitation does not match, SortFunction has been integrated into Collection in Squeak.
Is extracting sources From .mcz and filtering in change list still possible? (Epicea?)
I will try and import in Pharo. But it was not humanely possible yesterday night.


> 2017-11-06 10:13 GMT+01:00 Denis Kudriashov <[hidden email]>:
> It is difficult to compare to current Pharo version.
> Why you not produce Pharo pull request?
>
> 2017-11-06 0:06 GMT+01:00 Nicolas Cellier <[hidden email]>:
> I've put some of the proposed composition features into
>
> http://source.squeak.org/inbox/Collections-nice.766.mcz
> http://source.squeak.org/inbox/CollectionsTests-nice.283.mcz
>
> http://source.squeak.org/inbox/Collections-nice.766.diff
> http://source.squeak.org/inbox/CollectionsTests-nice.283.diff
>
> SInce it's Squeak based, it uses <=>, but never mind, that's the same functions.
>
>
> Stephane: the original comments were from Travis Griggs.
> It's more a tutorial for using the SortFunction than a detailed explanation of implementation.
>
> The original idea is that oSortFunction are composable.
> For example, it's possible to chain sorting on a first criterion, then resort on a second criterion if objects have same rank with first.
>
> For this, we need a to distinguish when objects have same rank (=) and cannot use a binary result (Boolean) like legacy sortBlock,
> So we rather need a ternary comparison (<=>).
>
> This thread is about extending composition, and generalizing implementation by composition (like Xtreams)
> - for sorting nil first (or last)
> - for reversing the order
> - for sorting properties with any collation order, and not just default <=>
>
> 2017-11-05 17:52 GMT+01:00 Stephane Ducasse <[hidden email]>:
> Hi guys
>
> Do you have some nice comments somewhere? because I do not understand
> anything about this thread.
> I check the code and the comments are well... unclear and confusing.
>
> Stef
>
> On Sun, Nov 5, 2017 at 4:15 PM, Nicolas Cellier
> <[hidden email]> wrote:
> >
> >
> > 2017-11-05 16:06 GMT+01:00 Denis Kudriashov <[hidden email]>:
> >>
> >>
> >> 2017-11-05 11:33 GMT+01:00 Nicolas Cellier
> >> <[hidden email]>:
> >>>
> >>>
> >>> 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
> >>
> >>
> >> I like your idea.
> >> It also forced me to think that direction itself should be implemented as
> >> wrapper. I would name it InvertedSortFunction:
> >>
> >> InvertedSortFunction>>collate: value1 with: value2
> >>     ^(actualSortFunction collate: value1 with: value2) * -1
> >>
> >>
> >> If we will do it then direction will be not part of SortFunction. And all
> >> current functions will be in fact ascending.
> >> And to explicitly reflect this fact I would introduce
> >> AscendingSortFunction as their superclass.
> >> InvertedSortFunction and ChainedSortFunction will stay subclasses of
> >> SortFunction.
> >>
> >> So what you think?
> >
> >
> > Yes, I was thinking the same.
> > On another hand, direction makes thing symmetric and has its elegance too.
> > What I don't like with it is that it forces the library to have two
> > different messages for the same thing:
> > - collate:with: in base class accounting for direction
> > - threeWayCompare:with: in every subclass
> >
> > The fact to hardcode the direction in base class could be seen as an
> > optimization too.
> > I'm not sure what Sista optimization could bring in this case, because the
> > selectors may get megamorphic...
> >
> >
> >>
> >>>
> >>>
> >>> 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
> >>>
> >>
> >
>
>
>
>



Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

tinchodias


On Mon, Nov 6, 2017 at 10:20 AM, Nicolas Cellier <[hidden email]> wrote:


2017-11-06 10:51 GMT+01:00 Sven Van Caekenberghe <[hidden email]>:


> On 6 Nov 2017, at 10:37, Nicolas Cellier <[hidden email]> wrote:
>
> Because it's a cross dialect library and because contributing in Squeak is so much easier for me.
>
> I like the social power provided by github and i know how to use git, at least as an expert beginner.
> But it took me two hours to pick a pharo image, fork and clone the pharo repository, pick a pharo VM (I ended up building my own),
> retrieve my ssh pass phrase to avoid using github thru https,
> search documentation of where should I put the image wrt to my cloned git repository,
> search how to use iceberg,
> and finally realized that the author/date would not even be preserved if we want to port this cross dialect library.

You are absolutely right, but it is improving and will get better.


Nope, i don't buy the argument...
The information has simply been erased and won't reappear by operation of wishfull thinking
Even if we later add the blame API, the information of original authors is definitely lost.

That was already the case at each Pharo release, the mc ancestry was reset and it was very difficult to trace back.
7.0 is in continuity, just with a more radical eradication.

For Pharo, such compatibility is a drag, for me it's usefull, it's just that we have different trade offs
It's not for complaining, but trade offs are to be made explicit and assumed.


> The dark theme upsets me, especially the green on blue, I could have searched how to change it, but there I stopped.

Then switch to the light/white theme, just one setting.

> While doing all these things, I did not spent a minute focusing on the SortFunction subject.
> I can see that as an investment, maybe I should have done it before, but it was too much for a Sunday evening.

I think what Denis means is, it would have been helpful to apply the change in Pharo. How you communicate that is less relevant, just saving the MC version and mailing it would already have been very helpful.

IMHO.

Right, I can understand that cross dialect is becoming inconvenient...
That's why I gave the .diff URL just for giving a chance to glimpse at refactoring POC.

Pharo still has Monticello, so it could have worked (not sure how well without common ancestor)
The extra difficulty here is that package delimitation does not match, SortFunction has been integrated into Collection in Squeak.

Hi, punctually about this:
 
Is extracting sources From .mcz and filtering in change list still possible? (Epicea?)

Yes, in latest Pharo 6 and 7 you can file browse or drag&drop the source.st inside the .mcz to either file-in or see the code. (But no, it's not Epicea).

Martin
 
I will try and import in Pharo. But it was not humanely possible yesterday night.


> 2017-11-06 10:13 GMT+01:00 Denis Kudriashov <[hidden email]>:
> It is difficult to compare to current Pharo version.
> Why you not produce Pharo pull request?
>
> 2017-11-06 0:06 GMT+01:00 Nicolas Cellier <[hidden email]>:
> I've put some of the proposed composition features into
>
> http://source.squeak.org/inbox/Collections-nice.766.mcz
> http://source.squeak.org/inbox/CollectionsTests-nice.283.mcz
>
> http://source.squeak.org/inbox/Collections-nice.766.diff
> http://source.squeak.org/inbox/CollectionsTests-nice.283.diff
>
> SInce it's Squeak based, it uses <=>, but never mind, that's the same functions.
>
>
> Stephane: the original comments were from Travis Griggs.
> It's more a tutorial for using the SortFunction than a detailed explanation of implementation.
>
> The original idea is that oSortFunction are composable.
> For example, it's possible to chain sorting on a first criterion, then resort on a second criterion if objects have same rank with first.
>
> For this, we need a to distinguish when objects have same rank (=) and cannot use a binary result (Boolean) like legacy sortBlock,
> So we rather need a ternary comparison (<=>).
>
> This thread is about extending composition, and generalizing implementation by composition (like Xtreams)
> - for sorting nil first (or last)
> - for reversing the order
> - for sorting properties with any collation order, and not just default <=>
>
> 2017-11-05 17:52 GMT+01:00 Stephane Ducasse <[hidden email]>:
> Hi guys
>
> Do you have some nice comments somewhere? because I do not understand
> anything about this thread.
> I check the code and the comments are well... unclear and confusing.
>
> Stef
>
> On Sun, Nov 5, 2017 at 4:15 PM, Nicolas Cellier
> <[hidden email]> wrote:
> >
> >
> > 2017-11-05 16:06 GMT+01:00 Denis Kudriashov <[hidden email]>:
> >>
> >>
> >> 2017-11-05 11:33 GMT+01:00 Nicolas Cellier
> >> <[hidden email]>:
> >>>
> >>>
> >>> 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
> >>
> >>
> >> I like your idea.
> >> It also forced me to think that direction itself should be implemented as
> >> wrapper. I would name it InvertedSortFunction:
> >>
> >> InvertedSortFunction>>collate: value1 with: value2
> >>     ^(actualSortFunction collate: value1 with: value2) * -1
> >>
> >>
> >> If we will do it then direction will be not part of SortFunction. And all
> >> current functions will be in fact ascending.
> >> And to explicitly reflect this fact I would introduce
> >> AscendingSortFunction as their superclass.
> >> InvertedSortFunction and ChainedSortFunction will stay subclasses of
> >> SortFunction.
> >>
> >> So what you think?
> >
> >
> > Yes, I was thinking the same.
> > On another hand, direction makes thing symmetric and has its elegance too.
> > What I don't like with it is that it forces the library to have two
> > different messages for the same thing:
> > - collate:with: in base class accounting for direction
> > - threeWayCompare:with: in every subclass
> >
> > The fact to hardcode the direction in base class could be seen as an
> > optimization too.
> > I'm not sure what Sista optimization could bring in this case, because the
> > selectors may get megamorphic...
> >
> >
> >>
> >>>
> >>>
> >>> 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
> >>>
> >>
> >
>
>
>
>




Reply | Threaded
Open this post in threaded view
|

Re: about SortFunctions

Stephane Ducasse-3
In reply to this post by Nicolas Cellier
Hi nicolas

While I know the ascending and other protocol, it is still unclear to
me the composition aspect. Did Travis write a tutorial
can we can use as documentation/comment?

BTW
"Example: #('abc'  'de' 'fghi') sorted: #size asscending"
=>

"Example: (#('abc'  'de' 'fghi') sorted: #size) ascending
=> show the result"

Stef

PS: In Pharo we do not remove history of methods for the fun.
I can tell you that I do not really like Git but I do not want our
language to be looking like a terrible system just because it does not
use a modern versioning control system.

I do not expect that we will move to another versioning control system
in the near future so we will be able to take advantage of the git
infrastructure.
And versioning code with resources and decent branch support will be
game changing.




On Mon, Nov 6, 2017 at 12:06 AM, Nicolas Cellier
<[hidden email]> wrote:

> I've put some of the proposed composition features into
>
> http://source.squeak.org/inbox/Collections-nice.766.mcz
> http://source.squeak.org/inbox/CollectionsTests-nice.283.mcz
>
> http://source.squeak.org/inbox/Collections-nice.766.diff
> http://source.squeak.org/inbox/CollectionsTests-nice.283.diff
>
> SInce it's Squeak based, it uses <=>, but never mind, that's the same
> functions.
>
>
> Stephane: the original comments were from Travis Griggs.
> It's more a tutorial for using the SortFunction than a detailed explanation
> of implementation.
>
> The original idea is that oSortFunction are composable.
> For example, it's possible to chain sorting on a first criterion, then
> resort on a second criterion if objects have same rank with first.
>
> For this, we need a to distinguish when objects have same rank (=) and
> cannot use a binary result (Boolean) like legacy sortBlock,
> So we rather need a ternary comparison (<=>).
>
> This thread is about extending composition, and generalizing implementation
> by composition (like Xtreams)
> - for sorting nil first (or last)
> - for reversing the order
> - for sorting properties with any collation order, and not just default <=>
>
> 2017-11-05 17:52 GMT+01:00 Stephane Ducasse <[hidden email]>:
>>
>> Hi guys
>>
>> Do you have some nice comments somewhere? because I do not understand
>> anything about this thread.
>> I check the code and the comments are well... unclear and confusing.
>>
>> Stef
>>
>> On Sun, Nov 5, 2017 at 4:15 PM, Nicolas Cellier
>> <[hidden email]> wrote:
>> >
>> >
>> > 2017-11-05 16:06 GMT+01:00 Denis Kudriashov <[hidden email]>:
>> >>
>> >>
>> >> 2017-11-05 11:33 GMT+01:00 Nicolas Cellier
>> >> <[hidden email]>:
>> >>>
>> >>>
>> >>> 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
>> >>
>> >>
>> >> I like your idea.
>> >> It also forced me to think that direction itself should be implemented
>> >> as
>> >> wrapper. I would name it InvertedSortFunction:
>> >>
>> >> InvertedSortFunction>>collate: value1 with: value2
>> >>     ^(actualSortFunction collate: value1 with: value2) * -1
>> >>
>> >>
>> >> If we will do it then direction will be not part of SortFunction. And
>> >> all
>> >> current functions will be in fact ascending.
>> >> And to explicitly reflect this fact I would introduce
>> >> AscendingSortFunction as their superclass.
>> >> InvertedSortFunction and ChainedSortFunction will stay subclasses of
>> >> SortFunction.
>> >>
>> >> So what you think?
>> >
>> >
>> > Yes, I was thinking the same.
>> > On another hand, direction makes thing symmetric and has its elegance
>> > too.
>> > What I don't like with it is that it forces the library to have two
>> > different messages for the same thing:
>> > - collate:with: in base class accounting for direction
>> > - threeWayCompare:with: in every subclass
>> >
>> > The fact to hardcode the direction in base class could be seen as an
>> > optimization too.
>> > I'm not sure what Sista optimization could bring in this case, because
>> > the
>> > selectors may get megamorphic...
>> >
>> >
>> >>
>> >>>
>> >>>
>> >>> 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
>> >>>
>> >>
>> >
>>
>

123