Fix-it fix-it fix-it!

[squimby]

I don't mean 'fix it' in general, as I'm sure there are plenty of people with that request. Specifically I'm looking for help with the interpreter becoming upset with $userhash{$currow[0]}{$domain[$i]} = 1;, and $userhash{$currow[0]}{$domain[$i]} = "undef"; and I'm getting the warning 'requires explicit package name', though that should be within the scope of that variable's declaration, right? Thoughts? Advice? Rants?

#!/usr/bin/perl
use strict;
use warnings;

print "What is the name of the access rights .csv? (Full file name)\n"
+;
my $file = <>;
print "Is there a header in this file?\n";
my $header = <>;
if ($header =~ m/^yes|^y]/i){
    $header = "1";
}
else {$header = "undef";
}
print "What file format are your messages in?\n";
my $format = <>;

open (CSV, "<", $file);
my (@emailList, @domain, %userHash);
while (my $currow = <CSV>){
    if ($header || $currow =~ m/^email|^e-mail/i){
        @domain = split (',', $currow);
        shift @domain;
        $header = "undef";
    }
    if($currow =~ m/[_a-zA-Z0-9-]+(\.[_a-zA-Z0-9-]+)*@[a-zA-Z0-9-]+(\.
+[a-zA-Z0-9-]+)*\.(([0-9]{1,3})|([a-zA-Z]{2,3}))$/){
        for my $i (0..$#domain){
            my @currow = split (',', $currow);
            if ($currow[$i+1]) {
                $userhash{$currow[0]}{$domain[$i]} = 1;
            }
            else{
                $userhash{$currow[0]}{$domain[$i]} = "undef";    
            }
            push (@emailList, $currow[0]);
        }
    }
    else{
        print STDERR "This email address isn't properly formatted! $!\
+n"
    }
}

[davido]

Perl's identifiers are case sensitive. %userHash is not the same as %userhash.

One of the first things I do in such situations is a global case-insensitive search for the identifier using my editor. As I iterate through the search results it's much easier for my eyes to spot when something changes than by just skimming through the code.

One thing that might have made it easier to spot would be following Schwern's Skimmable Code talk's advice. Moving some of the complexity in your while/if constructs into subroutines would move the usage of %userhash visually closer to its declaration, which might be a good thing.

[squimby]

Pardon my french, but dammit! I feel like an idiot. Removing this thread.

[davido]

The thread won't be removed (it doesn't meet the criteria for reaping), and please don't obliterate the node's content. It's a harmless learning experience that others may benefit from as well.

[jwkrahn]

print "What is the name of the access rights .csv? (Full file name)\n"
+;
my $file = <>;

That would be better written as:

print "What is the name of the access rights .csv? (Full file name)\n"
+;
chomp( my $file = <STDIN> );





print "Is there a header in this file?\n";
my $header = <>;
if ($header =~ m/^yes|^y]/i){
    $header = "1";
}
else {$header = "undef";
}

At this point $header will ALWAYS be true.    I think you meant $header = undef; instead of $header = "undef";.





open (CSV, "<", $file);

You should always verify that the file opened correctly before trying to use an invalid filehandle.

open CSV, '<', $file or die "Cannot open '$file' because: $!";

[Tux]

All good advice :)

And of course add to this that one should use Text::CSV_XS or Text::CSV to read/parse CSV. The method in the OP might work now, but is unlikely to work when the CSV gets real.

[jmlynesjr]

Hey! The post had value. It gave me a problem to search for that didn't involve a 75 character regex! Thanks!

James

[sundialsvc4]

Indeed, never obliterate an entry, and, when you find the solution to your problem (even if it involves egg on your face), “write it up.”   A thread, even one that is several years old, is or should be a permanent resource.   Make it find-able, and make it complete.   We’ve all done such-as-this.   Someday, someone who “did it too” will, perhaps silently, thank you.

[Anonymous Monk]

Indeed, I'll say that too!