Question regarding variable scope

[jpfarmer]

I'm working on a script to produce player pairings for my little brother's chess club. I've written a subroutine produce a pairing.

What I don't understand is the way lexical scoping works for $black_player. I declare it with my at the beginning of the routine, fill it in the foreach loop, and then when I get the value I want, I use last to jump out to the return. However, the value at the return statement always has the initial value.

I've observed the same result when I declare the variable with our as well. It seems like the foreach loop is using local internally. Is that the case? and is there a way to get the behavior I'm looking for, short of using some kind of intermediate variable?

# This takes a player, then determines a valid game pairing.  It 
#returns the opponent's name if there is one, otherwise
# it returns undef.
sub find_paring($white_player){
    my $white_player = $_[0];
    my $black_player = "none";

    # Get a list of players still in the running
    my @possible_opponents = 
        keys %{$matches_remaining{$white_player}};

    # Shuffle the list to add some randomness, then find the first
    # avaliable opponent.
    &shuffle(\@possible_opponents);
    PAIRING: foreach $black_player (@possible_opponents){
        # Once we have a valid match, we remove the entries from the
        # match hash, and return the pair.
        if (exists $matches_remaining{$black_player}{$white_player}){
            delete $matches_remaining{$white_player}{$black_player};
            delete $matches_remaining{$black_player}{$white_player};
            last PAIRING;
        }
    }

    # Return the info we have
    return ($white_player, $black_player);
}

[Roger]

The answer is yes, and I have modified your code slightly, the code below should work -



sub find_paring()
{
    my $white_player = shift;
    my $black_player = "none";

    # Get a list of players still in the running
    my @possible_opponents =
        keys %{$matches_remaining{$white_player}};

    # Shuffle the list to add some randomness, then find the first
    # avaliable opponent.
    &shuffle(\@possible_opponents);
    foreach (@possible_opponents)
    {
        # Once we have a valid match, we remove the entries from the
        # match hash, and return the pair.
        if (exists $matches_remaining{$_}{$white_player}){
            $black_player = $_;
            delete $matches_remaining{$white_player}{$_};
            delete $matches_remaining{$_}{$white_player};
            last;
        }
    }

    # Return the info we have
    return ($white_player, $black_player);
}

[Rhandom]

Hmmmm....

sub find_paring()
{
  my $white_player = shift;

Looks like a conflict of interest there. Can't really shift if you aren't allowing anything to be passed. Try

sub find_paring
{
  my $white_player = shift;

I think you'll be happier.

[Roger]

Umm, I have to disagree with you here. The code


sub find_parsing()
{
  ...
}

is equivalent to


sub find_parsing
{
  ...
}

I always start my subroutines with sub function() (a habit from shell programming days). Strict argument prototyping is not necessary with Perl though.

[Enlil]

I've observed the same result when I declare the variable with our as well. It seems like the foreach loop is using local internally. Is that the case?

Take a look at perldoc perlsyn (the section on Foreach) for more information but the short answer is yes.

and is there a way to get the behavior I'm looking for, short of using some kind of intermediate variable?

As Roger shows above just change your looping variable, and set $black_player to the value you before you exit the loop.

-enlil

[chimni]

When you declare my $black_player at the start of the sub .That variable is global through the scope of the sub .
(My or our would behave the same with respect to this sub,behaviour would vary with respect to code followinng down the line).
When you do a foreach $black_player (@possible_opponents)
This effectively overrides the above $black_player .It is local to the loop.Same principle as a pass by value in C.
Therefore ,you can rename your looping variable or just do what roger has done ,use $_.

[MarkM]

Actually, my() and our() do not behave the same from a subroutine. Both define a lexical context for the variable, however, my() allocates a new variable each time it is evaluated, while our() reuses a global.

For recursive subroutines, or subroutines that are accessed at the same time from multiple threads, my() variables are safe, while our() variables are unsafe.

[delirium]

Another option is to move the return statement up to the loop, where $black_player has been localized. Then you can explicitly return undef on a failed match like the function comments state. Here's my slightly condensed (and untested) rewrite:

sub find_paring() {
  my $wp = $_[0];
  foreach my $bp (&shuffle keys %{$matches_remaining{$wp}}) {
    if (exists $matches_remaining{$bp}{$wp})  {
      delete $matches_remaining{$wp}{$bp};
      delete $matches_remaining{$bp}{$wp};
      return ($wp, $bp);
    }
  }
  # Else return undef
  undef;
}

[hanenkamp]

This is being a little picky, but I noted you used:

sub find_pairing($white_player){ ... }

in the code snippet. This prototype will work, but isn't strictly correct. It would be better to say:

sub find_pairing($) { ... }

I also noted that some of the replies used:

sub find_pairing() {
    my $white_player = shift;
    ...
}

This is bad. If you call this method after this declaration, then a call like find_pairing("Bob") will fail because the prototype will accept no arguments. It would be better to say nothing about the signature of the subroutine like:

sub find_pairing { ... }

Anyway, you're all probably aware of this, but I wanted to make sure the casual browser, who might not, knows the difference between these slight variations. I look forward to Perl 6, where this cruft will be cleaned up.

[MarkM]

Better yet, avoid the use of prototypes altogether in Perl. They are broken when it comes to actual programmer expectations. $ means "force scalar context" not "expect a scalar". Note that find_pairing(@list) does not generate a compile time error, but will generate perhaps-unexpected runtime behaviour.