Global Symbol requires package name while using "my" in Sub

Question

I have a class where we are learning Perl so forgive me if I made a simple/obvious error as I am still learning. My question is why do I get the error

Global Symbol "%localhash" requires explicit package name

as well as the same error for "$param" when I do declare it with "my %localhash" the first time inside the Sub.

My code for reference:

use strict;
use warnings;
use Exporter;
use vars qw(@ISA @EXPORT);

@ISA=qw(Exporter);
@EXPORT=("insert_user", "modify_user", "remove_user", "generate_list");


#Insert user Function
Sub insert_user{
    my $param = shift;
    my %localhash = %$param;
    print "Please enter Username: ";
    my $user_name = <>;
    chomp($user_name);
    if(exists ($localhash{$user_name})){ 
        print "Error, user already exists!";
        last;
    }
    $user_name =~ s/[^a-zA-Z0-9]//g;
    $user_name = lc($user_name);
    $localhash{$user_name};
    return %localhash;
    print "Please enter a password: ";
    my $user_password = <>;
    %localhash{$user_name} = $user_password;
    return %localhash;
}

In my class we are suppose to use "my $param" and "my %localhash" various times so I would repeat the same process in declaring "my" in every Sub but I keep getting the same error as if the "my" is not there.

`%localhash{$user_name} = $user_password;` - this line should be `$localhash{$user_name}`, no? And why do you basically break the code with than `return %localhash;` in the middle of it? ) — raina77ow, Sep 22 '15 at 18:17
I am unsure about that, I followed my professor for that line. Not sure if he made a mistake or if I followed incorrectly there but I dont think I got an error for that. — Robert Rodriguez, Sep 22 '15 at 18:21
@ysth I will be calling this subroutine inside a loop from my main Perl file, is this format still fine if I want it to exit the loop if user does not exist? — Robert Rodriguez, Sep 22 '15 at 18:22
@RobertRodriguez: that will work, but is a very bad idea and so gives a warning; to suppress the warning, do `if (...) { print "Error, user already exists!"; no warnings 'exiting'; last; }` — ysth, Sep 22 '15 at 18:34

ysth · Accepted Answer · 2015-09-22T23:18:45.353

4

The first error you get is syntax error at foo.pl line 13, near "my "; getting a syntax error throws perl's parser off and you will often get spurious errors afterwards due to it not having successfully recognized a declaration or being mistaken about scope. Ignore them and fix the syntax error (which in this case is actually a couple lines earlier: Sub instead of sub, as pointed out by toolic). That will get you on to the next error :)

Other comments:

Using last as you do is bad; it will exit a loop, but there isn't a loop in your code. If your sub is called from within a loop, it will (with a warning) exit your sub and exit that loop, but that is the kind of action at a distance that leads to bugs.

For a long time, Exporter hasn't needed to be subclassed; simply use Exporter 'import'; instead of setting @ISA.

edited Sep 22 '15 at 23:18

answered Sep 22 '15 at 18:25

ysth

96,171
6
121
214

Thank you guys so much! Yes, the problem was that since it was Sub rather than sub and not continuing on, also found a few more little errors thanks to fixing this. Thank you again. – Robert Rodriguez Sep 22 '15 at 18:28
In that case what would be my best bet to have the program restart should the `if(exists ($localhash{$user_name}))` = true? – Robert Rodriguez Sep 22 '15 at 18:31
I would probably make it just do `return;` and in the caller, do something like `%user = insert_user(...) or last;` – ysth Sep 22 '15 at 18:36
Thank you very much once again, I just realized with what you said that the `return %localhash;` is what that is for. – Robert Rodriguez Sep 22 '15 at 18:40

score 0 · Answer 2 · answered Sep 22 '15 at 19:19

In addition to the issue with Sub, you have %localhash{$user_name} instead of $localhash{$user_name}; you check whether your user name already exists before you strip non-alphanumeric characters and lower-case it, which is wrong; and you fail to chomp the password. You also have very little blank space, which can make a program much more readable

This is something like what I would write

use strict;
use warnings;

use Exporter qw/ import /;

our @EXPORT = qw/ insert_user modify_user remove_user generate_list /;

sub insert_user {
    my ($param)   = @_;
    my %localhash = %$param;

    print "Please enter user name: ";
    my $user = <>;
    chomp $user;

    $user =~ s/[^a-zA-Z0-9]//g;
    $user = lc $user;
    if ( exists $localhash{$user} ) {
        warn "Error, user already exists!";
        return;
    }

    print "Please enter a password: ";
    my $pass = <>;
    chomp $pass;
    $localhash{$user} = $pass;

    return %localhash;
}

Global Symbol requires package name while using "my" in Sub

2 Answers2