Writing a container module

[yoda54]

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");

[chromatic]

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?

[Tanktalus]

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.