On Oct 5, 2009, at 3:19 53PM, Nicolas Cellier wrote:
2009/10/5 Henrik Johansen <henrik.s.johansen@veloxit.no>:
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.
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 :)
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. Cheers, Henry