Validate newline delimited email list

philosophia has asked for the wisdom of the Perl Monks concerning the following question:

This code is supposed to split up a newline delimited list of email addresses, and iterate through each line, using Email::Valid to test each address. It is not working correctly - it seems to be flagging every address as being invalid, even correct addresses.

my ($error) = 0;
    my (@other_addresses) = split( /\n/, $email_list );
        foreach (@other_addresses) {
                chomp($_);
                print $_;
                $error++ unless(Email::Valid->address($_));
        }
    $error_message .= "$error Email list contains an invalid email add
+ress.<br>" unless( $error < 1 );
[download]

Comment on Validate newline delimited email list Download Code

Replies are listed 'Best First'.
Re: Validate newline delimited email list by japhy (Canon) on Jan 12, 2007 at 21:08 UTC
First of all, use `split /\n/`. Don't put a literal newline in your code -- that's ugly. Second, the `chomp`ing is entirely unnecessary if you're splitting the string on newlines. Third, are you entirely sure the email addresses contain validly formatted addresses and nothing else? I'd suggest doing: `foreach (@other_addresses) { my $ok = Email::Valid->address($_); print "address($_) = $ok\n"; }` [download] Jeff `japhy` Pinyan, P.L., P.M., P.O.D, X.S.: Perl, regex, and `perl` hacker How can we ever be the sold short or the cheated, we who for every service have long ago been overpaid? ~~ Meister Eckhart	[reply] [d/l] [select]
Re: Validate newline delimited email list by ikegami (Patriarch) on Jan 12, 2007 at 21:14 UTC
Works for me except I had to initialize `$email_list`, initialize `$error_message` and print `$error_message`. Could you provide a snippet that actually shows the error? There are a couple of details worth mentioning. `split( /\n/, ... )` [download] is easier to read than `split( / /, ... )` [download] `... if( $error > 0 );` [download] or just `... if $error;` [download] is easier to read than `... unless( $error < 1 );` [download]	[reply] [d/l] [select]
Re^2: Validate newline delimited email list by philosophia (Sexton) on Jan 12, 2007 at 21:35 UTC
the current version of this code looks like `my ($error) = 0; my (@other_addresses) = split( /\n/, $email_list ); foreach (@other_addresses) { my $ok = Email::Valid->address($_); $error++ if(!$ok); print "address($_) = $ok\n"; print "error ".$error."<br>"; } $error_message .= "$error Email list contains an invalid email ad +dress.<br>" if $error;` [download] and gives me `address(test@test.com ) = error 1 address(test2@test.com) = test2@test.com error 1 There was an error with your form submission. 1 Email list contains an invalid email address.` [download] it seems to let every address through whether it is valid or not.	[reply] [d/l] [select]
Re^3: Validate newline delimited email list by ikegami (Patriarch) on Jan 12, 2007 at 21:50 UTC
`"test@test.com "` is indeed bad. Notice the trailing space? It's easy to fix the address, though. `sub trim { my ($s) = @_; for ($s) { s/^\s+//; s/\s+$//; return $_; } } ... my @other_addresses = map trim, split( /\n/, $email_list ); ...` [download]	[reply] [d/l] [select]
Re^4: Validate newline delimited email list by philosophia (Sexton) on Jan 12, 2007 at 22:20 UTC
Re^4: Validate newline delimited email list by philosophia (Sexton) on Jan 12, 2007 at 22:44 UTC
Re^5: Validate newline delimited email list by ikegami (Patriarch) on Jan 12, 2007 at 22:52 UTC
Re^4: Validate newline delimited email list by philosophia (Sexton) on Jan 12, 2007 at 22:08 UTC
Re^5: Validate newline delimited email list by ikegami (Patriarch) on Jan 12, 2007 at 22:31 UTC
Some notes below your chosen depth have not been shown here
Re^3: Validate newline delimited email list by Sagacity (Monk) on Jan 12, 2007 at 22:53 UTC
Looks to me that this line $error++ if(!$ok); is merely testing whether $ok exists, which it always does according to the line above it. `my $ok = Email::Valid->address($_);` [download] Try testing for the value of $ok that was returned instead. `if ($ok == 1) {#the rest of your code } else {# code here too if you like }` [download]	[reply] [d/l] [select]
Re: Validate newline delimited email list by ikegami (Patriarch) on Jan 12, 2007 at 20:48 UTC
Something's wrong with what you posted. Some lines are missing (where is `$error` set?) and the `print` line is cut halfway through.	[reply] [d/l] [select]
Re^2: Validate newline delimited email list by philosophia (Sexton) on Jan 12, 2007 at 20:51 UTC
$error is set in the first line of code. I've revised the print line. Thanks for taking a look...	[reply]
Re^3: Validate newline delimited email list by ikegami (Patriarch) on Jan 12, 2007 at 21:05 UTC
`$error` is set in the first line of code. You know I meant where it set after being initialized because you added `$error++ unless(Email::Valid->address($_));` [download]	[reply] [d/l] [select]
Re: Validate newline delimited email list by MonkE (Hermit) on Jan 12, 2007 at 21:53 UTC
Are you using Perl 5.0.0.4 or newer? Because it's a preqrequisite mentioned on CPAN. What happens if you evaluate Email::Valid->address('redstone@nasa.gov') or some other string constant, email address that you know to be valid? I ask because perhaps you are having character encoding issues on your form submission.	[reply]
Re^2: Validate newline delimited email list by philosophia (Sexton) on Jan 12, 2007 at 22:06 UTC
perl 5.8.5. 'redstone@nasa.gov' evaluates correctly, as in Email::Valid->address('redstone@nasa.gov') returns the email address. An invalid email address returns nothing.	[reply]
Re: Validate newline delimited email list by phaylon (Curate) on Jan 16, 2007 at 12:54 UTC
Just as a quick note, I would do: `my @addrs = split /\n/, $email_list; my @addrs_invalid = grep { not Email::Valid->address($_) } @addrs; print 'The following Email addresses are invalid:<ul>' . (map { '<li>' . $_ . '</li>' } @addrs_invalid) . '</ul>';` [download] Untested and only proof of concept (no encoding and stuff), of course ;) Ordinary morality is for ordinary people. -- Aleister Crowley	[reply] [d/l]

Back to Seekers of Perl Wisdom