Global symbol requires explicit package name

[cunningrat]

Relevant code snippets:

use strict;
use warnings;
...
foreach my $date (@dates)
       {
        foreach my $line (@{$maint_hash{$address}})
          {
            my (@alljobs,@alljobids,@received);
            my %rtoa;
            runsingle($line,$date,\*OUT);
          }
       }
...
}

sub runsingle {
... some code that actually populates the declared variables...
printjob($fh,$h,@foo);
...
}

sub printjob {
    my $fh=$_[0];
    my $h=$_[1];
    shift @_;
    shift @_;
    foreach my $key (@_)
    {
       print $fh $h->open('tr');
       my $key2=$rtoa{$key};
       print $fh $h->td(${$alljobs[$key2]}[2],${$alljobs[$key2]}[3],${
+$alljobs[$
key2]}[4]);
       my ($chopdate,$choptime,$junk)= split " ",${$received[$key]}[4]
+;
       $chopdate=~s/-201.//;
       print $fh $h->td("Ran at $choptime",$chopdate,${$received[$key]
+}[3]);
       print $fh $h->close('tr');
     }
    print $fh $h->close('table');
}

When I run this code, it complains that global symbols "%rtoa" and "@alljobs" require an explicit package name. The complaints are coming from inside the printjob subroutine. The variables are declared with "my" in the inner loop inside the main program, which means that their scope is limited to that inner loop. The ONLY place runsingle is called from is inside that inner loop, and the ONLY place printjob is called from is inside runsingle, so I'd think both printjob and runsingle should see those variables as "global".

Moving the declaration inside the runsingle subroutine doesn't change the behavior.

I don't understand why it's complaining. Would some kind soul explain it to me, please?

I could get around the error by passing those arrays and hashes as parameters... but as you can see from the printjob code, the arrays are actually multidimensional, so the resulting code would be hairy and hard to maintain.

UPDATE: nevermind, got it to work by declaring the variables inside a block that contains both subroutines. It's not elegant, but it'll do.

[choroba]

You are confusing lexical and dynamic scope. See What's the difference between dynamic and lexical (static) scoping? Between local() and my()?

A subroutine should obtain the data through arguments, not variables.

[ColonelPanic]

To elaborate on what choroba said, the sub can see variables in the scope in which it is declared, not the scope in which it is called. In this case, printjob can only see variables that are either declared in the sub itself or in the outer scope of the file.

The "quick fix" would be to move those variable declarations outside the loops. However, as choroba said, the preferred solution is to pass everything that is needed into the sub.

In this case, it is not clear why %rtoa and @alljobs need to exist outside of runsingle. I would declare them there, then pass them in to printjob by reference:

sub runsingle {
my %rtoa;
my @alljobs;   
... some code that actually populates the declared variables...
printjob($fh,$h,\@foo,\%rtoa,\@alljobs);
...
}

sub printjob {
    my $fh=$_[0];
    my $h=$_[1];
    my $keys = $_[2];
    my $rtoa = $_[3];
    my $alljobs $_[4];

    foreach my $key (@{$keys})
    {
        ...

Also, the new style of filehandles is to use lexical variables directly. This eliminates the need for \*OUT:

    open my $out_file, '>', 'filename.txt' or die "Ouch, $!";

[greengaroo]

Sorry for jumping in, but I have to say something! To me it's as if someone scratches his nails on a chalkboard!

This:

    my $fh=$_[0];
    my $h=$_[1];
    shift @_;
    shift @_;
    foreach my $key (@_)

... is not clean!

Try this:

    my ( $fh, $h, @keys ) = @_;
    
    foreach my $key ( @keys ) {

Or this if you prefer:

    my $fh = shift;
    my $h = shift;
    
    foreach my $key ( @_ ) {

That's my 2 cents.

[ColonelPanic]

I agree that the original code is not clean. However, your solution makes an unnecessary copy of the array. You could just do this

    my $fh=shift;
    my $h=shift;

    foreach my $key (@_)

Though this issue disappears entirely if the sub is switched to pass by reference in order to fix the other problems.

[greengaroo]

Exactly! If you know you will pass a large structure to a subroutine, better pass it as a reference! Like this:

# Calling routine
routine ( $v1, $v2, \@array );

sub routine {
my ( $fh, $h, $keys ) = @_;
...
# Dereference the array in a loop:
foreach my $key ( @{$keys} ) {

[cunningrat]

Thank you all for the replies. Even though I figured it out myself, your comments were VERY helpful as far as helping me write cleaner, better code goes.