[squeak-dev] The Inbox: Collections-mt.846.mcz

Levente Uzonyi leves at caesar.elte.hu
Mon Jul 22 16:52:17 UTC 2019


Hi Marcel,

Dictionary >> #add: and Dictionary #associationsDo: are dangerous methods.
The former takes its argument and optionally integrates it into the 
dictionary. The latter exposes the internal structure of the 
dictionary.

Therefore, it is risky to #add: associations from one dictionary to 
another, because the dictionaries may share these associations, and that 
will result in hard-to-debug situations.

Here's a snippet that should just work (works without your changes, fails 
with them):

| original copy |
original := Dictionary new.
original at: 1 put: 2.
copy := original collect: [ :each | each ] as: Dictionary.
self assert: (copy at: 1) = 2.
original at: 1 put: 3.
self assert: (copy at: 1) = 2.

And here is a snippet simulating the problem (always fails):

| original copy |
original := Dictionary new.
original at: 1 put: 2.
copy := Dictionary new.
original associationsDo: [ :each | copy add: each ].
self assert: (copy at: 1) = 2.
original at: 1 put: 3.
self assert: (copy at: 1) = 2

I suggest Dictionary >> #fillFrom:with: be changed to the following to 
achieve what you want:

fillFrom: aCollection with: aBlock
 	"Evaluate aBlock with each of aCollections's elements as the argument.
 	Collect the resulting values into self. Answer self."

 	aCollection isDictionary
 		ifFalse: [
 			aCollection do: [ :element |
 				self add: (aBlock value: element) ] ]
 		ifTrue: [
 			aCollection associationsDo: [ :association |
 				self at: association key put: (aBlock value: association value) ] ]

This should also make things a bit faster in most cases. ;-)

Levente

On Mon, 22 Jul 2019, commits at source.squeak.org wrote:

> A new version of Collections was added to project The Inbox:
> http://source.squeak.org/inbox/Collections-mt.846.mcz
>
> ==================== Summary ====================
>
> Name: Collections-mt.846
> Author: mt
> Time: 22 July 2019, 3:34:37.445967 pm
> UUID: 32e5abfd-9153-4a57-b404-7c11380ccbd0
> Ancestors: Collections-mt.845
>
> Here is an idea to make this work:
>
> #(1 1 2 2 3 4) asSet collect: [:ea | ea -> ea] as: Dictionary
>
> See inbox CollectionsTests-pre.314.mcz. Not as fast as before, though. :-)
>
> =============== Diff against Collections-mt.845 ===============
>
> Item was changed:
>  ----- Method: Collection>>fillFrom:with: (in category 'private') -----
>  fillFrom: aCollection with: aBlock
>  	"Evaluate aBlock with each of aCollections's elements as the argument.
>  	Collect the resulting values into self. Answer self."
> 
> + 	aCollection associationsDo: [ :each |
> - 	aCollection do: [ :each |
>  		self add: (aBlock value: each) ]!
>
> Item was removed:
> - ----- Method: Dictionary>>fillFrom:with: (in category 'private') -----
> - fillFrom: aCollection with: aBlock
> - 	"Evaluate aBlock with each of aCollections's elements as the argument. 
> - 	Collect the resulting values into self. Answer self."
> - 
> - 	aCollection isSequenceable
> - 		ifTrue:
> - 			[aCollection associationsDo:
> - 				[ :element | self add: (aBlock value: element)]]
> - 		ifFalse:
> - 			[aCollection keysAndValuesDo:
> - 				[ :key :value | self at: key put: (aBlock value: value)]]!
>
> Item was added:
> + ----- Method: Dictionary>>histogramOf: (in category 'converting') -----
> + histogramOf: aBlock
> + 
> + 	^ self collect: [:assoc | aBlock value: assoc value] as: Bag!


More information about the Squeak-dev mailing list