[Nitro] OG [PATCH RFC]: Hash.dup -> deep copy function intended?
john at oxyliquit.de
Wed Sep 12 09:26:50 EDT 2007
> Looking over some of the OG code I see a fair amount of `Hash#dup`, where
> typically the Hash is some options hash passed in to the method.
> Some times options are updated/deleted/altered.
> I'm assuming the intent of the Hash#dup is to isolate any changes to
> these options (if not, I'd be interested to know what value .dup
> adds), but my understanding is that this (use of .dup) won't be the
> case with Hash.dup, and a deep copy is required.
'isolate changes to the options' is the right sentence.
The Hashes are the options, and thus the Hash must be protected.
The contents of the Hashes are (as far as I can remember, been a
while since I last looked at it) never changed.
There might be .update/.merge!/.deletes on the Hash, but since
that doesn't affect the contents, the options are 'isolated'.
So, while a deep copy might be the _really_ correct way, it is
more expensive than simply duplicating the Hash.
> My Q is:
> Is this right or is there some other reason for using Hash#dup?
> Is Marshal.load(Marshal.dump(hash)) the preferred (for performance)
> deep copy method?
I couldn't look at your patch yet, but I'm not sure if it's worth
doing a deep copy. The .dups right now (I added some as well) are
kind of "carefully crafted" to not lead to weird side effects.
Of course, if you can find the case of some string or something
else modified, just do tell. Then we might think again about
using either a deep copy or just recoding the modifying code.
I'm kinda late, but hope that helps,
Using Opera's revolutionary e-mail client: http://www.opera.com/mail/
More information about the Nitro-general