Re: [Seaside Commits] Seaside 3.0: Grease-Pharo-Core-lr.15.mcz

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

Re: [Seaside Commits] Seaside 3.0: Grease-Pharo-Core-lr.15.mcz

Julian Fitzell-2
So why was the test failing?

On Wed, Mar 31, 2010 at 7:41 AM,
<[hidden email]> wrote:

> Lukas Renggli uploaded a new version of Grease-Pharo-Core to project Seaside 3.0:
> http://www.squeaksource.com/Seaside30/Grease-Pharo-Core-lr.15.mcz
>
> ==================== Summary ====================
>
> Name: Grease-Pharo-Core-lr.15
> Author: lr
> Time: 31 March 2010, 8:41:10 am
> UUID: 4cc58271-ef9c-4e96-91aa-d37e9e3d82f4
> Ancestors: Grease-Pharo-Core-jok.14
>
> - fix test for SortedCollection>>#sorted: by removing some code, not really sure if this is a valid fix but it makes all tests pass
>
> =============== Diff against Grease-Pharo-Core-jok.14 ===============
>
> Item was changed:
>  ----- Method: SequenceableCollection>>sorted: (in category '*grease-pharo-core') -----
>  sorted: sortBlock
> +       ^ self sortBy: sortBlock!
> -       ^ (self sortBy: sortBlock) as: self species!
>
>
> _______________________________________________
> commits mailing list
> To unsubscribe, email [hidden email]
> http://lists.seaside.st/listinfo/commits
>
_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Seaside Commits] Seaside 3.0: Grease-Pharo-Core-lr.15.mcz

Lukas Renggli
> So why was the test failing?

>>  ----- Method: SequenceableCollection>>sorted: (in category '*grease-pharo-core') -----
>>  sorted: sortBlock
>> +       ^ self sortBy: sortBlock!
>> -       ^ (self sortBy: sortBlock) as: self species!

#species of SortedCollection is SortedCollection. Sorting a
SortedCollection with a non-default sortBlock, but then the collection
is converted to a SortedCollection with a default sort block, what
gives the default sorting.

I don't know if it is correct to remove the #as: and what was its
intention? It doesn't break any tests though, so I guess it is fine.
Alternatively we could implement the method in SortedCollection like
this.

Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch
_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Seaside Commits] Seaside 3.0: Grease-Pharo-Core-lr.15.mcz

Julian Fitzell-2
On Wed, Mar 31, 2010 at 9:42 AM, Lukas Renggli <[hidden email]> wrote:

>> So why was the test failing?
>
>>>  ----- Method: SequenceableCollection>>sorted: (in category '*grease-pharo-core') -----
>>>  sorted: sortBlock
>>> +       ^ self sortBy: sortBlock!
>>> -       ^ (self sortBy: sortBlock) as: self species!
>
> #species of SortedCollection is SortedCollection. Sorting a
> SortedCollection with a non-default sortBlock, but then the collection
> is converted to a SortedCollection with a default sort block, what
> gives the default sorting.
>
> I don't know if it is correct to remove the #as: and what was its
> intention? It doesn't break any tests though, so I guess it is fine.
> Alternatively we could implement the method in SortedCollection like
> this.

I think the tests are not thorough enough then, though I thought I had
tests for the class of the returned object (maybe we removed them
because they were failing?). But it does seem like #sorted:, which
returns a copy, should be returning an instance of #species (certainly
this is necessary for classes like Interval, which has a species of
Array).

Certainly the current implementation of #sorted is inefficient and
suboptimal and could be improved on SortedCollection to simply "^ self
copy sortBlock: aBlock; yourself".

Julian
_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Seaside Commits] Seaside 3.0: Grease-Pharo-Core-lr.15.mcz

Paolo Bonzini-2
On 03/31/2010 12:24 PM, Julian Fitzell wrote:

>>>> >>>    sorted: sortBlock
>>>> >>>  +       ^ self sortBy: sortBlock!
>>>> >>>  -       ^ (self sortBy: sortBlock) as: self species!
>> >
>> >  #species of SortedCollection is SortedCollection. Sorting a
>> >  SortedCollection with a non-default sortBlock, but then the collection
>> >  is converted to a SortedCollection with a default sort block, what
>> >  gives the default sorting.
>> >
>> >  I don't know if it is correct to remove the #as: and what was its
>> >  intention? It doesn't break any tests though, so I guess it is fine.
>> >  Alternatively we could implement the method in SortedCollection like
>> >  this.
> I think the tests are not thorough enough then, though I thought I had
> tests for the class of the returned object (maybe we removed them
> because they were failing?). But it does seem like #sorted:, which
> returns a copy, should be returning an instance of #species (certainly
> this is necessary for classes like Interval, which has a species of
> Array).

Wouldn't that be  ^(self as: self species) sortBy: sortBlock?

Paolo
_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Seaside Commits] Seaside 3.0: Grease-Pharo-Core-lr.15.mcz

John O'Keefe-2
I believe Paolo is correct.  #sort: is supposed to return the receiver sorted in-place while #sorted: is supposed to return a sorted copy of the receiver.  A comment to this effect would be nice also :-)
 
John
On Wed, Mar 31, 2010 at 6:31 AM, Paolo Bonzini <[hidden email]> wrote:
On 03/31/2010 12:24 PM, Julian Fitzell wrote:
>>>    sorted: sortBlock
>>>  +       ^ self sortBy: sortBlock!
>>>  -       ^ (self sortBy: sortBlock) as: self species!
>
>  #species of SortedCollection is SortedCollection. Sorting a
>  SortedCollection with a non-default sortBlock, but then the collection
>  is converted to a SortedCollection with a default sort block, what
>  gives the default sorting.
>
>  I don't know if it is correct to remove the #as: and what was its
>  intention? It doesn't break any tests though, so I guess it is fine.
>  Alternatively we could implement the method in SortedCollection like
>  this.
I think the tests are not thorough enough then, though I thought I had
tests for the class of the returned object (maybe we removed them
because they were failing?). But it does seem like #sorted:, which
returns a copy, should be returning an instance of #species (certainly
this is necessary for classes like Interval, which has a species of
Array).

Wouldn't that be  ^(self as: self species) sortBy: sortBlock?

Paolo

_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev



--
John O'Keefe [|], Principal Smalltalk Architect, Instantiations Inc.

_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev