yoda54 has asked for the wisdom of the Perl Monks concerning the following question:

Dear Monks,

I'm writing a generic container module but I can't seem to pinpoint some strange issues with the list method. What am I overlooking?

Thanks for any help!

package _Init; use strict; use warnings; sub new { my ($class, %args) = @_; my $self = bless { }, ref($class) || $class; $self->_init(%args); return $self; } 1; package Container; use _Init; @Container::ISA = qw(_Init); use strict; use warnings; sub _init { my ($self, %args) = @_; $self->{_default} = []; } sub add { my $self = shift @_; my $key = shift @_; my $target = shift @_; my $aref = $self->{$key}; push @$aref, $target; } sub addtype { my $self = shift @_; my $key = shift @_; $self->{$key} = []; } sub list { my $self = shift @_; my $key = shift @_; my $aref = $self->{$key}; #return $aref wont work as well #why is $aref broken? my $count = 0; foreach(@$aref) { print "$_\n"; $count++; } } sub remove { my $self = shift; my $type = shift @_; my $target = shift @_; my $aref = $self->{$type}; my $count = 0; foreach(@$aref) { if ($target eq $_) { splice @$aref, $count, 1; } $count++; } } 1; package main: $self = Container->new(); $self->addtype("_population"); $self->add("_population", $someobjref); $self->list("_population");

Replies are listed 'Best First'.
Re: Writing a container module
by chromatic (Archbishop) on Aug 07, 2006 at 06:56 UTC

    Define "broken". After fixing a couple of copy and paste errors, it does what I think it should do. What do you think it should do? What does it do for you?

Re: Writing a container module
by Tanktalus (Canon) on Aug 07, 2006 at 14:07 UTC

    Given that you don't really say how you want it to work, I'm going to just make a bunch of unwarranted assumptions - perhaps they'll be right.

    First, you probably want the list method to be:

    sub list { my $self = shift @_; my $key = shift @_; my $aref = $self->{$key}; return wantarray ? @$aref : $aref; }
    That assumption is that you were trying to retrieve it via code like:
    my @population = $self->list("_population");

    I'm also presuming you haven't tested (or tested very well) your remove function. I'm curious as to what happens if there is more than one object that equals the target. My guess is "no good". Much simpler is:

    sub remove { my $self = shift; my $type = shift @_; my $target = shift @_; $self->{$type} = [ grep { $target ne $_ } @{$self->{$type}} ]; return; }
    I'm also quite curious as to why you "shift @_" most of the time, but "shift" once. Since it's such a common idiom in perl, I'd suggest removing the @_ from all shift's.

    Of course, if this isn't the solution to your query, you'll need to be a bit more specific as to the problem.