The Inbox: Collections-mt.846.mcz

Am 22.07.2019 18:52:28 schrieb Levente Uzonyi <[hidden email]>:

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, [hidden email] 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!