1

I have been working on a cgi file that will check if a username is already taken when a user wants to register their credentials. If the username is taken it is supposed to notify them, if not it saves their credentials to the original flat file. I am having trouble comparing variables that I have assigned value to within a foreach statement. I am telling the foreach to assign a username to a variable if the name that a user input is the same as one that is already stored. I have it assigning the variables properly, but after words I want to tell it to compare those variables again outside of the foreach so the operation is only being done once. Here is my current code

#!/usr/bin/perl use warnings; use strict; use CGI qw(:standard); use CGI::Carp qw/fatalsToBrowser warningsToBrowser/; use Digest::MD5 qw(md5 md5_hex md5_base64); #telling what variables are still to be used as global our ($username, ,$user, $nametaken); #assigning some local variables my $username = param("username"); my $password = param("password"); my $hashpass = md5_hex($password); print header, start_html(); #creating an array from the flatfile that usernames and passwords are stored my @users = do { open my $fh, "<", "password.txt" or die $!; map { chomp; split /:/ } <$fh> }; #comparing the values in the array to the username entered foreach my $user (@users) { if ($user eq $username) { #printing here to test if it is comparing correctly which it is print p("$user\n"); #assigning the $user value to $nametaken so it can be compared to later my $nametaken = $user; #printing here to test if the variable was correctly assigned, which it is print p("$nametaken\n"); } } #printing here to test if the variable was correctly assigned, which it is not printing #so the foreach must be causing some king of issue for this variable after it is done and I don't know what that is print p("$nametaken\n"); #Here is where I am trying to check if the username already exists and then save the user credentials if it does not if ($nametaken eq $username) { print p("Username already taken, Try again"); } #As of now the else statement is running everytime and saving new user credentials even if a username is already taken else { open my $fh, ">>", "password.txt" or die $!; print $fh "$username:$hashpass\n"; print p("Your account has been created sucessfully"); close $fh; } print end_html(); 
2
  • Ah, our friend the For-If Antipattern. Commented Feb 12, 2012 at 21:21
  • Why are you using qw(...) and qw/.../, just pick one. I would actually recommend qw'...', or qw"..." for use on StackOverflow, as it highlights better. Commented Feb 14, 2012 at 6:09

3 Answers 3

4

You are declaring a NEW lexically scoped variable $nametaken inside of your foreach loop - or rather, inside the if {} block: my $nametaken = $user;

It may share the same name with $nametaken variable you had outside, but it's a completely different variable, with the scope of that if block - once you exit the if, that variable is completely forgotten. Whatever value you assigned to it is lost.

You can see more details about lexically scoped variables here:

http://perldoc.perl.org/perlsub.html#Private-Variables-via-my%28%29


To solve your problem tactically, you simply need to remove the my declaration from within the if: $nametaken=$user;

To do it correctly, in a Perl way, you should re-think your approach to the problem alltogether. You CAN use a foreach loop to detect if a value is in the list, but it's definitely NOT the best (readability wise, or sometimes even performance wise) Perl technique. A more idiomatic way is to use hash lookups:

my %users = map { ($_ => 1) } @users; # Create a hash with users being keys if ($users{$username}) { print "$username already taken!\n"; } 
Sign up to request clarification or add additional context in comments.

5 Comments

Awesome, I changed my code and implemented the hash lookups. Worked perfect. Thanks! Can you explain the hash setup up a little more. When you write map { ($_ => 1) } , what is this saying?
@Salmonerd - I can :) You may want to make it a separate question - it's a somewhat distinct topic that may be useful for other users
@DVK- Maybe you could take a look at my newest question about trying to use hash lookups for login verification. I would really like to understand them better and what I am trying to do with them is really above my level of experience. Thanks for your help thus far.
If you changed it to if(exists $users{$username}){...} then you could use my %users;@users{@users}=();
@Brad - neat! Even more idiomatic
3
my $nametaken = $user; 

creates a new variable named $nametaken that has nothing to do with the $nametaken you declared outside the loop.

1 Comment

Yep that was the problem, still pretty new to Perl and the scope of variables and how to declare them. Thanks for pointing out the obvious, I think I have my head wrapped around it a little better know.
2

The reason why $nametaken does not have a value outside the foreach loop is because it has been lexically scoped to be defined only inside the foreach.


There is always more than one way to do it :

my ( $nametaken ) = grep { /$username/ } @users; if ( $nametaken ) { ... } else { ... } 

or simply :

if ( grep { /$username/ } @users ) { ... } else { ... } 

It's usually less noisy when you have fewer temporary variables in Perl.

2 Comments

He already had our $nametaken outside the loop, so this would still give duplicate declarations.
@cjm : Ack. Didn't notice that. Irrelevant example removed.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.