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 |
> 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 |
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 |
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 |
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:
-- John O'Keefe [|], Principal Smalltalk Architect, Instantiations Inc. _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
Free forum by Nabble | Edit this page |