1

I have a module with the following subroutines:

package module;

sub new
{
    my $class = shift;
    my $reference = shift;
    bless $reference, $class;
    return $reference;
};

sub add_row{

    @newrow = [1,1,1];
    push @_, @newrow;

};

@matricies is an array of array references. I created objects out of the array references using

my @object= map{module->new($_)} @matrices;

and lets say I want to add a row to one of the objects using:

@object[0]->add_row();

I think there's something wrong with the add_row subroutine dealing with the use of @ and $. Any ideas?

Axeman
  • 29,660
  • 2
  • 47
  • 102
user1224478
  • 345
  • 2
  • 5
  • 15

4 Answers4

4

Yes, you are adding an array ref yet your variable is an array.

The first thing you need to do (after using strict and warnings) is to read the following documentation on sigils in Perl (aside from good Perl book):

The best summary I can give you on the syntax of accessing data structures in Perl is (quoting from my older comment)

  • the sygil represents the amount of data from the data structure that you are retrieving ($ of 1 element, @ for a list of elements, % for entire hash)

  • whereas the brace style represent what your data structure is (square for array, curly for hash).


Now, for your code:

@newrow = [1,1,1];
push @_, @newrow;

should be

my $newrow = [1,1,1];
push @_, $newrow;

You have another problem in how to access your rows from the object:

sub add_row {
    my ($self, $newrow) = @_;
    $newrow ||= [1,1,1]; # In case it wasn't passed, default to this?
    push @{$self}, $newrow;
};

You also have the same problem with @object[0]->add_row(); as with newrow - you are using array sigil to address 1 element

$object[0]->add_row(); # will add default row of [1,1,1]
$object[0]->add_row([2,3,4]);

UPDATE: here's a complete code:

Module's add_row() (your constructor is fine):

sub add_row {
    my ($self, $newrow) = @_;
    $newrow ||= [1,1,1]; # In case it wasn't passed, defauly to this?
    push @{$self}, $newrow;
};

Test driver:

use a1;
my @objects = (a1->new([[5,6,7],[8,9,10]]));
$objects[0]->add_row();
$objects[0]->add_row([3,4,5]);
use Data::Dumper; print Data::Dumper->Dump([\@objects]);

Results:

$VAR1 = [
      bless( [
               [
                 5,
                 6,
                 7
               ],
               [
                 8,
                 9,
                 10
               ],
               [
                 1,
                 1,
                 1
               ],
               [
                 3,
                 4,
                 5
               ]
             ], 'a1' )
    ];
Community
  • 1
  • 1
DVK
  • 126,886
  • 32
  • 213
  • 327
  • I tried this, and it will add the array ref in the wrong place in the object. Instead of `[ [1,1,1], [1,1,1] ]` you get `[ 1,1,1, [1,1,1] ]`. – TLP Mar 06 '12 at 17:16
  • Aah, you added another array layer. I just did `..->new([1,2,3])`, in which case the error occurs. – TLP Mar 06 '12 at 17:34
2

Okay, back up.

First, you need to use strict; and use warnings;. Always. Each and every single time. If you don't, you're begging for trouble.

Also, @newrow = [1,1,1]; isn't correct, since @newrow denotes an array, but [1,1,1] is an array reference. Further, @newrow is only defined within your subroutine, so add_row doesn't really do anything. What you need is to pass a reference to the matrix to which you want to add a row.

What I think you're trying to do is model a matrix as an array of array references. So, for example, if we had

my @matrix=([1,0,0],[0,1,0],[0,0,1]);

Then that could be considered as a matrix with rows of [1,0,0], [0,1,0], and [0,0,1].

So, ignoring the idea of creating a module for the moment, what you're probably looking for is something like the following:

use strict;   #ALWAYS
use warnings; #ALWAYS

#array of three array references, each of which has three elements.
my @matrix=([1,0,0],[0,1,0],[0,0,1]);

#The arguments to add_row are (in order):
#1.  A reference to the matrix to which you want to add a row.
#2.  A list of the elements that you wish to add.

sub add_row
{
  my $matrix_arg=shift;
  my @new_row_array=@_;

  #Now, we do the necessary push:
  push @$matrix_arg,\@new_row_array;
}

#Now we can add a row and check whether or not we we are successful:

add_row(\@matrix,2,-17,5);

foreach my $row (@matrix)
{
  print join(",",@$row) . "\n";
}

The output is:

1,0,0
0,1,0
0,0,1
2,-17,5

Honestly, I'd recommend getting your hands on a copy of Learning Perl and also looking at perldoc perlref.

  • 1
    This is a good code but I think the answer is too cluttered - this guy does NOT need nuanced error handling when he has trouble with sigils – DVK Mar 06 '12 at 17:08
  • @DVK - Fair point. I tend to get carried away with error checking, even when prototyping very simple things. –  Mar 06 '12 at 17:10
1

Just a couple of things:

  • Use Local::Module instead of just module. The Local module space is reserved for non-CPAN modules. Also, by convention, Module names should start with an uppercase letter.
  • Use use strict and use warnings.

Let's look at your new subroutine constructor:

sub new
{
    my $class = shift;
    my $reference = shift;
    bless $reference, $class;
    return $reference;
};

I'm not sure what you're attempting to do here. Normally, this should be a constructor. You get a reference object back, you don't pass it a reference. Maybe you mean this?

package Local::Module;
sub new {
   my $class = shift;

   my $reference = {};
   bless $reference, $class;
   return $reference;
}

This wil create a NEW object you can use for adding. So, you'd do this first:

 my $object = Local::Module->new;

Now, you can use $object as a handle to your rows:

sub add_rows {
   my $self = shift;
   my $rowRef = shift;

   if (not exists $self->{ROWS}) {
       $self->{ROWS} = [];
   }
   push @{$self->{ROWS}}, $rowRef;
}

Now, you can use this object to add rows:

 my $object->add_row = $RowReference;

Notice that an object in Perl is usually a reference to an anonymous hash. You put the data you want in one of the keys of your hash. In this case, you put your array in $self->{ROWS}.

Yes, there are all sorts of ways to create pseudo hashes, and inside out hashes, but the idea is that your class is normally not the actual data, but a reference to an object that holds the data. Otherwise, you're not really using object oriented programming.

In your case, I wouldn't bother with map. I doubt it'll be more efficient, and a for loop will be cleaner (untested):

use strict;
use warnings;

my @matrices = ([1,0,0],[0,1,0],[0,0,1]);

my @objects;
foreach my $array_ref (@matrices) {
   my $module_ref = Local::Module->new;
   my $module_ref->add_row($array_ref);
   push @objects, $module_ref;
}


package Local::Module;
sub new {
   my $class = shift;

   my $reference = {};
   bless $reference, $class;
   return $reference;
}


sub add_rows {
   my $self = shift;
   my $rowRef = shift;

   if (not ref($rowRef) eq "ARRAY") {
       die qq(Method "add_rows" can only take an Array reference);
   }

   if (not exists $self->{ROWS}) {
       $self->{ROWS} = [];
   }
   push @{$self->{ROWS}}, $rowRef;
}

Now, your list of @objects is a list of Local::Module classes. You could now do something like this:

  $objects[2]->add_row($row_ref);

This is a very rough outline. For example, you probably want another module that will return a reference to all of your arrays. Maybe another that can pop and shift the rows in your array, and return the popped or shifted row.

You also might want to include a way to pass in the initial reference to an array:

sub new {
   my $class = shift;
   my $array_ref = shift;

   my $reference = {};
   bless $reference, $class;
   if (defined $array_ref) {
       $reference->add_row($array_ref);
   }
   return $reference;
}

Note that once I bless $reference, I can use it in object oriented calls. This way, my constructor has no idea what my object looks like. There's a general idea that constructor and methods should be ignorant of how the rest of the object is structured. That way, when you modify the object, you only have to modify one or two isolated methods. If I change the way my method add_rows work, I don't have to modify my constructor. The change is isolated in a single location.

David W.
  • 105,218
  • 39
  • 216
  • 337
0

@_ is a temporary variable used only to pass in arguments to the function, so putting arguments onto it doesn't modify the object itself.

Instead, you need to pull off the object from the @_ array and then modify it instead.

sub add_row{
    my $self = shift;
    @newrow = (1,1,1);
    push @$self, @newrow;
};
Wes Hardaker
  • 21,735
  • 2
  • 38
  • 69