[Nitro] OG [PATCH RFC]: Hash.dup -> deep copy function intended?

Mark Van De Vyver mvyver at gmail.com
Wed Sep 12 16:40:45 EDT 2007


Hi,

On 9/12/07, Jonathan Buch <john at oxyliquit.de> wrote:
> Hi,
>
> > 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've not seen anything yet - just paranoid - but prefer
performance,,,, so 'if it ain't broke.....'

There is a related issue you might want to opine on.
Should manager.options always be in sync with manager.store.options?
This relates to, what you think is the best way to 'expose' to
external users/code
the state (including options) that an adapter is in.... see my later email.
George is still considering this - unless something has been decided.

Thanks Mark

> I'm kinda late, but hope that helps,
>
> Jo
>
> --
> Using Opera's revolutionary e-mail client: http://www.opera.com/mail/
>


More information about the Nitro-general mailing list