2009/10/5 Henrik Johansen <[email protected]>:
>
> On Oct 5, 2009, at 3:19 53PM, Nicolas Cellier wrote:
>
>> 2009/10/5 Henrik Johansen <[email protected]>:
>>> Good to know I'm not the only one who found it strange :)
>>>
>>> BTW, I thought the default for Collections was to do a shallowCopy
>>> with a copy operation?
>>>
>>
>> I removed this default behaviour SequenceableCollection>>shallowCopy
>> which used to use (self copyFrom: 1 to: self size).
>> This beautifull hack was using self species, leading to this mess...
>> And it forced me defining a #basicShallowCopy to work the hack around
>> (which was the shortest but uggliest solution).
>>
>> The copy was deep enough so that modifying the copy would not modify
>> the original, and this is what postCopy now does.
>>
>>> ll := LinkedList new.
>>> ll add: Link new.
>>> ll2 := ll copy.
>>> ll add: Link new.
>>>
>>> inspecting ll2, it's firstLink does not have a nextLink, which
>>> implies
>>> to me a deepCopy has occured.
>>>
>>
| list link1 link2 link3 copy |
link1 := StackLink with: 1.0.
link2 := StackLink with: 2.0.
link3 := StackLink with: 3.0.
link4 := StackLink with: 4.0.
(list := LinkedList new) add: link1; add: link2; add: link3.
copy := list copy.
self assert: list size = 3.
self assert: copy size = 3.
self assert: copy first element == list first element.
list remove: link2.
self assert: list size = 2.
self assert: copy size = 3.
copy add: link4 before: copy third.
self assert: list size = 2.
self assert: copy size = 4.
^copy
As you can see, this is not a deepCopy, the element identity are preserved.
Of course not the link objects themselves, that can't be !
Otherwise modifying the copy would modify the original.
Like Associations in Dictionary, they should be copied (but the value
identity should not change).
>> I did not take time to write all tests yet, I will give it a try in
>> trunk.
>>
>>> Of course, it's arguable that that's better behaviour for
>>> LinkedLists,
>>> seeing as otherwise ll2's lastLink would be invalid when you added to
>>> ll.
>>> At a glance, SparseLargeTable, Stack (Through LinkedList
>>> implementation) and WideCharacterSet in your changeset all seem to do
>>> a deep copy.
>>>
>>
>> just deep enough, not more, not less.
>
> To my understanding, that's too deep.
> You're copying elements in the collection as well, not just creating a
> copy of the collection with the same elements.
> That's what I'd define as a deepCopy, after a copy, I'd expect the
> elements in them to be the same.
> Maybe it's just what I've gotten used to in VW, and Squeak does
> something else historically, but mixing and matching which to do based
> on what feels best at any given time can lead to some nasty bugs :)
>
I don't think so, at least not in trunk image.
The best thing would be to provide tests.
>>
>>> Also, how does part in SharedQueue2
>>> monitor critical: [
>>> monitor := Monitor new.
>>> items := items copy
>>> ] work? Seems a bit fishy to me to have a critical section on a
>>> monitor, then replacing it as part of it...
>>>
>>
>> He he, it should just work, because we are talking to a Monitor object
>> (the Smalltalk way), not to a pointer *monitor somewhere in memory
>> (the C/C++ view).
>
> Oh, absolutely, it will not crash due to deallocated pointers or such.
>
> What I meant was a concurrency issue, when some other object requests
> something that requires a monitor lock, after Monitor has been
> replaced, but before the items have been copied.
> i.e.
> while the intent of the critical section in that piece of code as far
> as I can tell, is that the two operations should happen without items
> being accessible before they have been copied.
> so you'd actually have to write something like:
> monitor critical: [
> monitor := Monitor new.
> monitor critical:
> [items := items copy]]
> to ensure that does not happen.
>
IMO, this is currently unnecessary because no one yet knows about this
copy. The sole use is
^self shallowCopy postCopy,
so there is no pointer on me yet but the active process stack.
We only have to protect against a modification of original in between
But in case of exotic use of #postCopy, you are right.
Just reversing the order would do.
monitor critical: [
items := items copy
monitor := Monitor new].
Nicolas
> Cheers,
> Henry
>
> _______________________________________________
> Pharo-project mailing list
> [email protected]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>
_______________________________________________
Pharo-project mailing list
[email protected]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
