Mantis - Squeak
Viewing Issue Advanced Details
6535 Collections minor always 06-09-07 16:45 04-18-10 22:04
R4p70r  
andreas  
normal  
closed 3.10  
fixed  
none    
none trunk  
0006535: keyBlock and sortBlock are lost when creating a collection of the same species.
Iím not sure if this is the expected behavior or not...

When a method in OrderedCollection or a KeyedSet return a new collection of itís own kind, that new collection rarely use the keyBlock or sortBlock of the original.
| element keyedSet |
element := Dictionary with: (#a->1).

"By default a KeyedSet will hash and compare the return value of #key on its elements. I want it to use (elem at: #a) instead "

keyedSet := KeyedSet keyBlock: [:elem | elem at: #a].
keyedSet add: element.

"keyedSet>>select attempts to return a new keyedSet without the sortBlock"
keyedSet select: [:elem | true]. "Dictionary doesNotUnderstand key"


| numbers reverseOrder firstThree |
numbers := #(1 2 3 4 5).
reverseOrder := SortedCollection sortBlock: [:x :y | x > y].
reverseOrder addAll: numbers.

"The elements are inverted"
self assert: [reverseOrder asArray = #(5 4 3 2 1)].

"Copy the first 3 elements"
firstThree := reverseOrder copyFrom: 1 to: 3.

"It appears to work"
self assert: [firstThree asArray = #(5 4 3)].

"but we have lost the sort block"
firstThree add: 1.

" firstThree is now #(1 5 4 3)! "
self assert: [firstThree asArray = #(5 4 3 1)] "fails"
related to 0007404closed nicolas cellier Use postCopy paradigm in collections 
 SortedCollection-LooseSortBlock-Test-M6535-nice.1.cs [^] (1,106 bytes) 05-22-08 08:01
 SortedCollection-LooseSortBlock-Patch-M6535-nice.1.cs [^] (2,314 bytes) 05-22-08 08:46
 KeyedCollection-LooseBlock-Test-M6535-nice.1.cs [^] (1,262 bytes) 05-22-08 12:57
 KeyedCollection-LooseBlock-Patch-M6535-nice.1.cs [^] (1,869 bytes) 05-22-08 12:58
 SortedCollection-LooseSortBlock-Patch-M6535-nice.2.cs [^] (2,385 bytes) 05-28-08 19:28
 M6535-Patch-Prereq-OrderedCollection-postCopyFromto.st [^] (860 bytes) 10-05-09 08:51
 M6535-Patch-OrderedCollection-postCopy.st [^] (202 bytes) 10-05-09 08:51
 M6535-KeyedCollection-Patch.1.cs [^] (1,500 bytes) 10-05-09 08:54

Notes
(0012139)
nicolas cellier   
05-22-08 08:53   
This is a critical bug and should have deserved more attention.

I uploaded a patch based on postCopy paradigm.
This patch rely on a shallowCopy... and SequenceableCollection>>shallowCopy would call copyFrom:to: loop infinitely.

To circumvent this, i moved Object>>shallowCopy code in basicShallowCopy and called basicShallowCopy.

Patch only address SortedCollection kind of copy methods, not KeyedCollection.
There might be other senders of species to track and correct.
(0012140)
nicolas cellier   
05-22-08 13:00   
And now a test and patch for KeyedCollection addressing select: bug.

Adopted solution is to define and use a copyEmpty in select:, just like OrderedCollection implementation.

It would be possible to move select: and copyEmpty implementation at Collection level.
(0012201)
nicolas cellier   
05-27-08 20:49   
"fix begin"
Installer mantis bug: 6535 fix:'SortedCollection-LooseSortBlock-Patch-M6535-nice.1.cs'.
Installer mantis bug: 6535 fix:'KeyedCollection-LooseBlock-Patch-M6535-nice.1.cs'.
"fix test"
Installer mantis bug: 6535 fix:'SortedCollection-LooseSortBlock-Test-M6535-nice.1.cs'.
Installer mantis bug: 6535 fix:'KeyedCollection-LooseBlock-Test-M6535-nice.1.cs'.
"fix end"
(0012211)
nicolas cellier   
05-28-08 19:31   
#(3 3) copyFrom: 2 to: 0 would raise a debugger, and that kind of bug just happens when opening a 'Message Names' interface.

So i had to protect OrderedCollection>>postCopyFrom:to: as a workaround.
(0012212)
nicolas cellier   
05-28-08 19:32   
"fix begin"
Installer mantis bug: 6535 fix:'SortedCollection-LooseSortBlock-Patch-M6535-nice.2.cs'.
Installer mantis bug: 6535 fix:'KeyedCollection-LooseBlock-Patch-M6535-nice.1.cs'.
"fix test"
Installer mantis bug: 6535 fix:'SortedCollection-LooseSortBlock-Test-M6535-nice.1.cs'.
Installer mantis bug: 6535 fix:'KeyedCollection-LooseBlock-Test-M6535-nice.1.cs'.
"fix end"
(0013147)
andreas   
07-03-09 06:42   
There is a major problem loading these changes. The change to OrderedCollection>>copyFrom:to: uses postCopyFrom:to: before that method is even defined. This screws up badly if it's ever called. Please split these change sets with the prerequisites coming first.
(0013148)
nicolas cellier   
07-03-09 19:37   
Yes, change sets order is definitely bogus.
I realized that recently when porting to
http://code.google.com/p/pharo/issues/detail?id=890 [^]
But did not have time to provide feedback here yet...
(0013339)
nicolas cellier   
10-05-09 00:43   
I'm not really satisfied with creating an Object>>#basicShallowCopy message.
I tested a far better solution: use the postCopy paradigm.
Defining #postCopy in a few subclasses, we can abandon SequenceableCollection>>#shallowCopy and thus avoid #basicShallowCopy.

This also apply to some unordered collections (Bag etc...) and enable removing a few #copy implementations. I will publish later.
(0013342)
nicolas cellier   
10-05-09 08:56   
"fix begin"
Installer mantis ensureFix: 7404.
Installer mantis bug: 6535 fix:'M6535-Patch-Prereq-OrderedCollection-postCopyFromto.st'.
Installer mantis bug: 6535 fix:'M6535-Patch-OrderedCollection-postCopy.st'.
Installer mantis bug: 6535 fix:'M6535-KeyedCollection-Patch.1.cs'.
"fix test"
Installer mantis bug: 6535 fix:'SortedCollection-LooseSortBlock-Test-M6535-nice.1.cs'.
Installer mantis bug: 6535 fix:'KeyedCollection-LooseBlock-Test-M6535-nice.1.cs'.
"fix end"
(0013343)
nicolas cellier   
10-05-09 08:57   
Fixed in http://source.squeak.org/trunk/Collections-nice.158.mcz [^]