passing a value to a function

[ralfthewise]

Hey all,
I'm not much of a OO programmer and this (I think pretty simple) function is giving me fits.

sub cleararray()
{
    my $class = shift;
    for ($i=0;$i<$radiovar;$i++)  #reset array to empty
    {
        for ($j=0;$j<$radiovar;$j++)
            {
                $array[$i][$j]="0";
            }
    }
}

So my problem is that I want to send to the function cleararray() the value that has been assigned to $radiovar in a different location, so that cleararray() can work on the data from the same starting point.
I've tried cleararray($radiovar) to get the value to cleararray() and I'm getting the error NOT ENOUGH ARGUMENTS FOR PATTERNS::CLEARARRAY. Patterns is the .pm, and cleararray is a subfxn.
thanks,
john

[bikeNomad]

This isn't C, and you don't need the parentheses after cleararray. In fact, they're hurting you, since by using them, you've told Perl that cleararray takes no arguments. Remove them.

Also, note that you're taking the first argument to cleararray and throwing it away, and then referring to a global. It might be better (depending on your program design) to pass in $radiovar. If you want to call cleararray as a class method, you can do this:

sub cleararray
{
  my $class = shift; # ignored
  my $radiovar = shift;
  ...
}
# then call as:
PATTERNS->cleararray($someRadioVar);

or if you are always calling it from within the same package it's defined in, perhaps:

sub cleararray
{
  my $radiovar = shift;
  ...
}
# then call as:
cleararray($someRadioVar);

[Sifmole]

Well cleararray() tell it to expect no arguements. You would need to do cleararray($)
You would also need to pick it up by changing my $class = shift; to  my ($self, $radiovar) = @_;

I believe I have heard that the prototyping in Perl is not really worth the effort; Especially when using OO because they are not checked in OO code.

[linux454]

Like the other monks have mentioned the parens after your function name are hurting you.
Another way of creating clearer OO code would be to encapsulate $radiovar an instance attribute of your class.
so you would have:


sub cleararray {
    my $self = shift;
    for ( $i = 0; $self->radiovar(); $i++ ) {
        for ( $j = 0; $self->radiovar(); $j++ ) {
            @{$self->array()}[$i][$j] = undef;
        }
    }
}

With this approach you would define two other accessor methods:

• one to return or set the value of $radiovar
• one to return a reference to the @array

so they might look something like this:

sub radiovar {
    my ( $self, $newval ) = @_;

    $self->{'radiovar'} = $newval if defined( $newval );

    return $self->{'radiovar'};
}

sub array {
    my ( $self, $newval ) = shift;
    $self->{'array'} = $newval if defined( $newval );
    return $self->{'array'};
}

This way you aren't using error prone global variables, but you are writing better OO code.
Not to mention when using global variables inside your objects it kind of defeats the purpose of writing OO in the first place,
in that when a variable's purpose changes or you need to do something different to present that variable then you have to go
change lots more code than if you only had to change one or two subroutines. (That last statement may need some clarification.)

Now if your situation calls for only one $radiovar value per class you could make it a class attribute pertaining to all object of Class X. Which would look something like:


package X;
...
my $radiovar;
...
sub radiovar {
   my $newval = shift;

   $radiovar = $newval if defined( $newval );
   return $radiovar;
}

[CharlesClarkson]

Since you already made some great points about calling and global variables, I'd like to add a bit about the algorithm. First the cleararray sub can be shortened:

sub cleararray {
    my $self = shift;
    @{$self->array()}[$_] = [(0) x $self->radiovar()]
           for 0 .. $self->radiovar() - 1;
}

Second, it may not be necesary to use $self->radiovar() if $self->array() is already defined for size:

sub cleararray {
    my $self = shift;
    @{$self->array()}[$_] = [(0) x @{$self->array()}]
           for 0 .. @{$self->array()} - 1;
}


HTH,
Charles K. Clarkson

[derek3000]

Ok, here goes nothing, my first post as an answer to someone. I hope I get this right...
It was said before that you don't need to declare the function with the parens. So:

sub cleararray{
#code
}

will do fine.

So as it stands, even if you did call it like:

cleararry($radiovar);

it still won't work, because the variable that you're popping off the argument list goes to $class, which is not what you want, because you wouldn't do anything with it. Also, the way it is now, you're just using a global variable ($radioval), so passing it isn't necessary. I am not saying that this is good!! I haven't done too much OO myself, but I think I would do it like this, but keep in mind, TMTOWTDI:

sub clear_array{
    my($self, $var) = @_;
    my @array;
    for(my $i = 0; $i < $var; $i++){
           for(my $j = 0; $j < $var; $j++){
                  $array[$i][$j] = 0;
           }
    }
    return @array;
}

I'm sure there's an easier way to do it. There's probably a function that will do this for you already. My version keeps all variables local to the function, which is usually a good practice (I think). But anyway, since it's like this, it returns a two-dimensional array with all of its values set to 0. So to call it:

my $leroy = patterns->new(); #or my $leroy = new patterns;
@array_to_be_cleaned = $leroy->clear_array($radioval);

If there really isn't any special reason that you're passing in $radioval, then you could just use foreach, I think. I hope this helps, and that I didn't mess up and confuse you more.

happy coding,
Derek

[ishmael]

ishmael

$array = [ ] ;

is sufficient. This will effectively "clear" the array. Depending on your requirements, $i=[ 0 ] , $i=undef ], and $i may be all equivalent.

Remember that undef is actually an array element. I have caught myself thinking that @array = (undef) is an empty array. Nope. It is a single-element array with the element undef!

[busunsl]

$array = [ ] ;

is sufficient. This will effectively "clear" the array.

Nope, it will not clear the array!
Instead it will create a new, empty anonymous array and store a reference to that in $array.

[ralfthewise]

Thanks for everyone's help. Actually cleararray is a little misleading. I need an array of zeros to get the program to fall through graphically. Is there a better algorithm to handle just that?
john
Faster, faster, faster....till the thrill of speed overcomes the fear of death -Daniel Keys Moran

[PrakashK]

# populate the array with zeroes

sub cleararray
{
    my ($class, $radiovar) = @_;
    my @array;
    push @array, [ (0) x $radiovar ] for 1 .. $radiovar;
    return \@array;
}