Bag>>deepCopy is not safe??

Folks,

I've encountered a very strange illogical situation while removing an
object from a deepCopied Bag.


bag := Bag new.
6 timesRepeat: [bag add: Object new].

copyBag := bag deepCopy.
copyBag remove: (copyBag detect: [:each| each isKindOf: Object]).
"ERROR: not found 'an Object'"



What I have leant from the walkback, is that Bag>>detect: invokes #do:
to find a match, while Bag>>remove: invokes Bag>>includesKey: which
search down a hash value of lookupTable inside the bag. Aha! I could
understand why this illogical thing is happening.
A deepCopied bag has a lookupTable contains hash values from the
original objects.
Since an Object's hash comes from its identiy not equality, loopTable
of copied bag has no chance to ever find its elements with it.


Silly but a clear example below works.


bag := Bag new.
6 timesRepeat: [bag add: Object new].

addAlled := Bag new.
addAlled addAll: bag.
addAlled remove: (addAlled detect: [:each| each isKindOf: Object]) "No
problem"



Does this mean we need to modify Bag behaviors?
Is this Dolphin 4.0 specific?

Howard Oh wrote:

> I've encountered a very strange illogical situation while removing an
> object from a deepCopied Bag.

I believe this is a bug in the implementation of Bag>>deepCopy.  A slightly
simpler example (which also demonstrates that it /must/ be considered a bug)
is:

    bag := Bag with: Object new with: Object new.
    copy := bag deepCopy.
    any := copy detect: [:each | true].
    copy includes: any.

which arbitrarily answers true or false.  The (mis)behaviour is the same in D5
and D6.

The actual implementation of Bag>>deepCopy is slightly odd (besides being
buggy) since it makes a copy of the contents dictionary twice.  The second time
it makes a copy it is not going through the normal copy path, and so omits the
necessary send of #rehash (or something similar).

    -- chris

"Chris Uppal" <[hidden email]> wrote in message
news:43c1030f$0$87292$[hidden email]...
Thanks guys, recorded as #2066.

Regards

Blair

I've added a test below to my unit test suite to see if the bug is
caught for the future.

DolphinTest>>testBagDeepCopy

        | bag copyBag exception |

        bag := Bag new.
        6 timesRepeat: [bag add: Object new].

        copyBag := bag deepCopy.
        [ copyBag remove: (copyBag detect: [:each| each isKindOf: Object]) ]
                on: Error do: [:e| exception := e].

        self assert: exception isNil



Maybe this is OA's job to do it right from the first place, who has the
best conception how the system works and how should they work.
Do you have plans to supply UnitTests for the very basic Dolphin
itself?
I'm pretty sure that OA keeps their closed unit test suite, for OA
being a XPer. :-)

Be it a full one or a piece, it will be very useful to me, since many a
time I hasitate to change the original codes of Dolphin package.
Modification is functionally required but fears bunch of bugs might
come back like a boomerang.


Thanks for adding my issue to a managed bug list.
And could anyone give a note when&how the bug is fixed for a poor
Dolpiner who sticks to version 4.01?

Best regards