in reply to Useless use of private variable in void context

Your immediate problem has been addressed and you've received a tip to improve your code within the narrow context of the code you've shown. However there are some 'bigger picture' items that should be mentioned.

First off, Perl has some really wizard stuff for handling arrays. grep and push turn the whole sub into:

sub AddToUserArray { my ($UserID) = @_; # Like this because it scales well for many par +ams # return 1 to skip addtouserlog () # return if bad UserID return 1 if $UserID eq ""; # return if handled user already return 1 if grep {$_ eq $UserID} @Participants; push @Participants, $UserID; return; # Generally better to return undef than 0 }

Note the last return which returns undef. Perl subs can return either a list or scalar value depending on calling context. undef works for most purposes in both contexts, but 0 becomes a 1 element list in list context and that can sometimes be surprising.

However, where you need to look stuff up arrays are often a bad choice. In Perl the better choice is to use a hash. The following code provides some assumed context for the sub to demonstrate how that goes:

use strict; use warnings; my %Participants; while (<DATA>) { chomp; my ($name, $tail) = split / /, $_, 2; next if ! defined ($name) || exists $Participants{$name}; $Participants{$name} = $tail; print "Added $name: $tail\n"; } __DATA__ Fred Red team Joe Blue Team Pete Blue Team Fred Red Team Annie Red Team Lee Green Team Steve Green Team

Note that the whole sub has been replaced by just two lines:

next if ! defined ($name) || exists $Participants{$name}; $Participants{$name} = $tail;

The more important, and less obvious advantage, is that exists works in pretty much constant (and very short) time - no more looking through the entire array each time you need to see if an entry is there already. For even modest numbers of entries that is an overwhelming factor!


True laziness is hard work

Replies are listed 'Best First'.
Re^2: Useless use of private variable in void context
by okarmi (Initiate) on Oct 13, 2009 at 17:33 UTC
    Dear GrandFather,

    Thank you very much for your answer. I know I have still much to learn.

    I actually thought about using a hash table but since this is my first script I wanted to keep it as simple (for me) as possible.

    I'm thinking about rewriting it and if so I'll use a hash table and the perlish loop style.

    Thank you again!