| Subject: | scalar context returns of list and hash give access to object internals |
This could be considered a bug in my assumptions, but it's not documented to behave the other way, so this is either a bug report or a feature request, depending on your point of view.
In scalar context, the list and hash methods return a reference to the contents of the list (or hash) field in the object. Unfortunately, this reference has to be treated as read-only, or as an invitation to break encapsulation.
#!/usr/bin/perl
package Foo; #traditional
use Class::MethodMaker new => "new", list => "a";
package main;
$f = Foo->new;
$f->a(1..3);
$aref = $f->a;
$last_item = pop @$aref; # oops, I just modified $f
__END__
The attached patch changes this behavior of the various *hash and *list methods to return a reference to an anonymous (shallow) copy of the field each time the method is called in scalar context. Appropriate patches to the static_hash and static_list tests are also included. (They'd be a lot easier with Test::More, but that's a different story.)
Advantages to this patch:
* prevent subtle bugs in user code.
* well, that's about it, really :-)
Disadvantages:
* minor performance hit
* could break code of people who used this property for quick
and dirty object modification. In which case, I personally think
they get what they deserve, but that's just me. :-)
diff -ur Class-MethodMaker-1.08/lib/Class/MethodMaker.pm Class-MethodMaker-1.0801/lib/Class/MethodMaker.pm
--- Class-MethodMaker-1.08/lib/Class/MethodMaker.pm Mon Oct 14 12:22:27 2002
+++ Class-MethodMaker-1.0801/lib/Class/MethodMaker.pm Tue Dec 31 13:17:48 2002
@@ -62,7 +62,7 @@
use Carp qw( carp cluck croak );
use vars qw( $VERSION $PACKAGE );
-$VERSION = '1.08';
+$VERSION = '1.0801';
$PACKAGE = 'Class-MethodMaker';
# ----------------------------------------------------------------------
@@ -954,7 +954,7 @@
# Use wantarray for consistency with list, which uses it for
# consistency with its own doco., and the hash impl.
- return wantarray ? @{$self->{$name}} : $self->{$name};
+ return wantarray ? @{$self->{$name}} : [ @{$self->{$name}} ];
};
$class->_add_list_methods(\%methods, $name);
@@ -1070,7 +1070,7 @@
# Use wantarray for consistency with list, which uses it for
# consistency with its own doco., and the hash impl.
- return wantarray ? @{$self->{$name}} : $self->{$name};
+ return wantarray ? @{$self->{$name}} : [ @{$self->{$name}} ];
};
$class->_add_list_methods(\%methods, $name);
@@ -1204,7 +1204,7 @@
: $obj_class->$new_meth(@$obj_class_ref) ;
}
}
- wantarray ? %{$self->{$name}} : $self->{$name};
+ wantarray ? %{$self->{$name}} : { %{$self->{$name}} };
};
$class->_add_hash_methods(\%methods, $name) ;
@@ -1687,7 +1687,7 @@
$self->{$field} = [ map { ref $_ eq 'ARRAY' ? @$_ : ($_) } @list ]
if @list;
- return wantarray ? @{$self->{$field}} : $self->{$field};
+ return wantarray ? @{$self->{$field}} : [ @{$self->{$field}} ];
};
$class->_add_list_methods(\%methods, $field);
@@ -1719,7 +1719,7 @@
$methods{$field} =
sub {
- return wantarray ? @storage : \@storage;
+ return wantarray ? @storage : [ @storage ];
};
@@ -1851,7 +1851,8 @@
@{$self->{$field}} = @_ if scalar @_ ;
- return wantarray ? @{$self->{$field}} : $self->{$field};
+ return wantarray ? @{$self->{$field}}
+ : [ @{$self->{$field}} ];
};
$class->_add_list_methods(\%methods, $field);
}
@@ -2029,7 +2030,7 @@
}
$self->{$field}->{$subkey} = $value;
}
- return wantarray ? %{$self->{$field}} : $self->{$field};
+ return wantarray ? %{$self->{$field}} : { %{$self->{$field}} };
} else {
cluck "Not a recognized ref type for hash method: $type.";
}
@@ -2044,7 +2045,7 @@
defined $value or carp "No value for key $key.";
$self->{$field}->{$key} = $value;
}
- return wantarray ? %{$self->{$field}} : $self->{$field};
+ return wantarray ? %{$self->{$field}} : { %{$self->{$field}} };
}
};
@@ -2599,7 +2600,7 @@
defined $value or carp "No value for key $key.";
$self->{$field}->{$key} = $value;
}
- wantarray ? %{$self->{$field}} : $self->{$field};
+ wantarray ? %{$self->{$field}} : { %{$self->{$field}} };
}
};
@@ -2642,7 +2643,7 @@
}
$hash{$subkey} = $value;
}
- return wantarray ? %hash : \%hash;
+ return wantarray ? %hash : { %hash };
} else {
cluck "Not a recognized ref type for static hash: $type.";
}
@@ -2657,7 +2658,7 @@
defined $value or carp "No value for key $key.";
$hash{$key} = $value;
}
- wantarray ? %hash : \%hash;
+ wantarray ? %hash : { %hash };
}
};
diff -ur Class-MethodMaker-1.08/t/static_hash.t Class-MethodMaker-1.0801/t/static_hash.t
--- Class-MethodMaker-1.08/t/static_hash.t Fri May 18 12:11:54 2001
+++ Class-MethodMaker-1.0801/t/static_hash.t Tue Dec 31 12:57:03 2002
@@ -72,7 +72,11 @@
TEST { ! defined $o->c('foo') };
TEST { defined $o->c };
-TEST { $o->a eq $o2->a };
+TEST { $ref_o = $o->a ; $ref_o2 = $o2->a;
+ $OK = keys %$ref_o2 == keys %$ref_o;
+ for (keys %$ref_0) { $OK = 0 unless $$ref_o{$_} eq $$ref_o2{$_};}
+ $OK;
+ };
exit 0;
diff -ur Class-MethodMaker-1.08/t/static_list.t Class-MethodMaker-1.0801/t/static_list.t
--- Class-MethodMaker-1.08/t/static_list.t Tue Oct 1 03:03:55 2002
+++ Class-MethodMaker-1.0801/t/static_list.t Mon Dec 30 22:02:43 2002
@@ -72,7 +72,11 @@
};
# 20--21
TEST { $@ };
-TEST { $o->a eq $o2->a };
+TEST { $ref_o = $o->a ; $ref_o2 = $o2->a;
+ $OK = @$ref_o2 == @$ref_o;
+ for (0..$#$ref_o) { $OK = 0 unless $$ref_o[$_] eq $$ref_o2[$_];}
+ $OK;
+ };
exit 0;