| Subject: | questions and optimizations |
| Date: | Mon, 15 Dec 2008 19:50:59 -0500 |
| To: | bug-hash-merge-simple [...] rt.cpan.org |
| From: | Uri Guttman <uri [...] stemsystems.com> |
hi,
i just found hash::merge::simple and i have some comments.
i think there is a subtle bug in how the hashes are copied/merged. the
left hash is copied at the top level but the lower level refs are the
same as in the left side. so if you modify left deep down after you
merged over it, it will modify both the left and the merged.
the returned hash ref is not the same as left but its guts may be shared
still. so either fix this or document that this is the behavior. you
aren't actually overlaying on left but it gives the impression you are
getting a complete copy of the merging. below is quick test which shows
this.
i intend to use this but i will have to do deep copies (which i already
have) before i do the merge. my data is filtered later on and if it is
shared between higher level things that could me filtering of filtered
data which is very bad. so knowing whether your module does deep copies
is important.
also i found some minor optimizations.
my $hr = (ref $right->{$key} || '') eq 'HASH';
my $hl = ((exists $left->{$key} && ref $left->{$key}) || '') eq 'HASH';
if ($hr and $hl){
ref works on undef values and doesn't generate a warning. and accessing
a missing key a the top level of a hash will not autovivify it so you
don't need the exists call either. the first two lines can be reduced
to:
my $hr = ref $right->{$key} eq 'HASH';
my $hl = ref $left->{$key} eq 'HASH';
and that can be reduced to:
my( $hr, $hl ) = map ref $_->{$key} eq 'HASH', $right, $left ;
or not. :)
so i will be copying the guts of this and making sure i do a deep copy
before i merge.
thanx,
uri
# this imported (actually pasted :) merge(). note the output shows that
# the top level foo hash is copied (the added aaa is only in left) but
# the 'bar' sub hash is modified in both left and merged.
use Data::Dumper ;
my $left = { foo => { bar => 2 } } ;
my $right = { baz => 4 } ;
my $merged = merge( $left, $right ) ;
print Dumper $left, $merged ;
$left->{foo}{bar} = 3 ;
$left->{foo}{aaa} = 5 ;
print Dumper $left, $merged ;
--
Uri Guttman ------ uri@stemsystems.com -------- http://www.sysarch.com --
----- Perl Code Review , Architecture, Development, Training, Support ------
--------- Free Perl Training --- http://perlhunter.com/college.html ---------
--------- Gourmet Hot Cocoa Mix ---- http://bestfriendscocoa.com ---------