subroutine reference parameters

[dnajjar]

Hi Perl Monks. I wrote this small portion of code to display the contents of a hash using a subroutine.

#!/usr/bin/perl -w
use warnings;

sub printElement(\%$)
{
   my %hash1 = %{($_[0])};
   my $key = $_[1];
   print $key." ".$hash1{$key}."\n";
}

my %hash = (0 => 'a', 1 => 'b', 2 => 'c');
while(($key) = each(%hash))
{
   &printElement(\%hash, $key);
}

I expected that the output to be something like

0 a
1 b
2 c

but instead, the program crashes trying to output infinitely "1 b". My knowledge about referencing is still not so good, and im working on it. Can someone kindly explain to me why this could be happening?

Thanks in advance.

Dawood

[lidden]

It seems using the hash as an argument in the function call makes it reset the internal iterator used by each, thats why you get an infinite loop.

Also you are declaring your function with a prototype, but when you call it you use an &. The & disables the prototype check.

[kennethk]

You have a number of errors interacting to result in your bug. The closest code to that you posted that functions is:

#!/usr/bin/perl -w
use warnings;

sub printElement(\%$)
{
   my $hash_ref = $_[0];
   my $key = $_[1];
   print $key." ".$hash_ref->{$key}."\n";
}

my %hash = (0 => 'a', 1 => 'b', 2 => 'c');
while(($key) = each(%hash))
{
   printElement(%hash, $key);
}

A large fraction of your problem can be traced back to incorrect use of Prototypes. For one thing, note that by using the & you are disabling the prototype. As well, if you hadn't disabled the prototype, passing in a hash reference would violate the prototype. In the end, your real problem is that using %hash in list context results in resetting the iterator in each. This is one of the reasons many people favor keys for the kind of loop you are describing.

As a side note, consider using strict: see Use strict warnings and diagnostics or die.

[AnomalousMonk]

In the end, your real problem is that using  %hash in list context results in resetting the iterator in each.

Or dereferencing a reference to  %hash in list context. E.g.:

>perl -wMstrict -le
"my %hash = qw(a 1  b 2  c 3);
 my $hashref = \%hash;
 ;;
 for (1 .. 3) {
   my ($k) = each %$hashref;
   print qq{'$k' => '$hashref->{$k}'};
   my @ra = %$hashref;
   }
"
'c' => '3'
'c' => '3'
'c' => '3'

Comment out the
    my @ra = %$hashref;
statement for more-expected behavior.

[choroba]

each returns two things, key and value. You only assign to $key. You probably wanted to use keys instead? Or this:

sub printElement {
    my ($key,$val) = @_;
    print $key,' ',$val,"\n";
}

my %hash = (0 => 'a', 1 => 'b', 2 => 'c');
while(my ($key,$value) = each(%hash)) {
    &printElement($key, $value);
}

[gnosti]

Hi,

You posted a link to the documentation for 'each'. Perhaps you should re-read it! When called in scalar context, the function returns a key. When called in list context, it returns a key-value pair.

[choroba]

OK, but ($key) = is not scalar context.

[toolic]

This doesn't answer your question as to why it is happening...

If you're not too fussy about how the hash's contents are formatted, you could just use the Core Data::Dumper module:

use warnings;
use strict;
use Data::Dumper;

my %hash = (0 => 'a', 1 => 'b', 2 => 'c');
print Dumper(\%hash);

__END__

$VAR1 = {
          '1' => 'b',
          '0' => 'a',
          '2' => 'c'
        };

In addition to arrays and hashes, this can be used for arbitrarily complex Perl data structures.

[Marshall]

As another way of dumping, I've been using pp() in Data::Dump lately. It gives a more compact display but basically same idea as Data::Dumper. example follows...

use warnings;
use strict;
use Data::Dump qw(pp);

my %hash = (0 => 'a', 1 => 'b', 2 => 'c');
print '%hash=',pp(\%hash),"\n";

__END__
Prints:
%hash={ "0" => "a", 1 => "b", 2 => "c" }

[jwkrahn]

sub printElement(\%$)
{
   my %hash1 = %{($_[0])};
   my $key = $_[1];
   print $key." ".$hash1{$key}."\n";

...

   &printElement(\%hash, $key);

You are copying the entire hash inside the subroutine so you may as well just do it like this:

sub printElement
{
   my ( $key, %hash1 ) = @_;
   print "$key $hash1{$key}\n";

...

   printElement( $key, %hash );

Of course, using a reference means that you don't have to copy the hash:

sub printElement
{
   my ( $hash1, $key ) = @_;
   print "$key $hash1->{$key}\n";

...

   printElement( \%hash, $key );

Also, if you use the prototype (\%$) then the first argument in the call to the subroutine has to be a hash and not a reference to a hash or you will get this error message:

Type of arg 1 to main::printElement must be hash (not reference constructor) at -e line 10

[Jenda]

Ouch! So you first specify a prototype for the printeElement subroutine (which you generally should not do unless you REALLY know what you are doing) and then call the subroutine with the & which turns the prototype off ... don't! Drop both the (\%$) and the & from the &printElement(....

Your problem is that the my %hash1 = %{($_[0])}; makes a copy of the hash and to make the copy it needs to iterate through the hash ... which resets the iterator position so the each() will always start from the beginning.

If you do insist on passing the key and the hash to your subroutine, do not make a copy! Work with the hash reference:

sub printElement
{
   my $hash1 = $_[0];
   my $key = $_[1];
   print $key." ".$hash1->{$key}."\n";
}

[dnajjar]

Thanks guys. I needed to understand the behavior and now I do. This is my first post on perlMonks website. Its amazing how you guys reply so fast! :) Thanks again!