Chop vs Chomp

[zoomcart.com]

Hello, and thanks in advance for your help. I have code (below) used
for adding emails from a user textbox to a flat file. The user is
asked to separate emails with a carriage return. Some times they will
use 2 carriage returns. The code is designed to separate emails by
returns, remove returns and then add them before writing. It is not
working perfectly. Some times chopping the last letter off, sometimes
not removing carriage returns. I'm sure there is a better way to do
this. Any help is appreciated.

sub bulk_emails
{
my ($newemail, $bademail, $email);
$checkemail = param('checkemail');
@emails= split(/\n/, $checkemail);
foreach $email(@emails){
chop $email;
chomp ($email) if ($email=~ /\n$/);
chomp ($email) if ($email=~ /\n$/);
unless ($email =~ /.*\@.*\..*/) {
$bademail=$email;
}
else{
$newemail .= "$email\n";
}
}

if($newemail){
open (USERS, ">>$userpath/lists/$list") || &error("$userpath/lists/
$list Update Account" , __FILE__, __LINE__,);
print USERS "$newemail";
close (USERS);
&success("Your Bulk emails have been added to the list");
}#new email
else{ &error("There were No REAL emails in your list");
}

 

[J. Gleixner]

Hello, and thanks in advance for your help. I have code (below) used
for adding emails from a user textbox to a flat file. The user is
asked to separate emails with a carriage return. Some times they will
use 2 carriage returns. The code is designed to separate emails by
returns, remove returns and then add them before writing. It is not
working perfectly. Some times chopping the last letter off, sometimes
not removing carriage returns. I'm sure there is a better way to do
this. Any help is appreciated.

use strict;
use warnings;

sub bulk_emails
{
my ($newemail, $bademail, $email);
$checkemail = param('checkemail');

my $checkemail = param('checkemail');

@emails= split(/\n/, $checkemail);
foreach $email(@emails){

for my $email ( split( /\n+/, $checkemail) ) {

or you can eliminate the variable:

for my $email ( split( /\n+/, param('checkemail') ) ) {

perldoc perlretut

"a+" = match 'a' 1 or more times, i.e., at least once


Then there's no need to chop/chomp anything.

To learn the difference:

perldoc -f chop
perldoc -f chomp

chop $email;
chomp ($email) if ($email=~ /\n$/);
chomp ($email) if ($email=~ /\n$/);

unless ($email =~ /.*\@.*\..*/) {

Not a terribly useful check since '@.' will match.

perldoc -q "How do I check a valid mail address"

$bademail=$email;

Which over-writes any previous value in $bademail.

}
else{
$newemail .= "$email\n";

Instead, you could use an array.

perldoc -f push

}
}

if($newemail){

open (USERS, ">>$userpath/lists/$list") || &error("$userpath/lists/
$list Update Account" , __FILE__, __LINE__,);

open( my $users, '>>', "$userpath/lists/$list" ) || error ("..." );

print USERS "$newemail";

print $users $newemail;

close (USERS); close ( $users );
&success("Your Bulk emails have been added to the list");
}#new email
else{ &error("There were No REAL emails in your list");
}

perldoc -q "What's the difference between calling a function as &foo and
foo()"

 

[Martijn Lievaart]

Hello, and thanks in advance for your help. I have code (below) used for
adding emails from a user textbox to a flat file. The user is asked to
separate emails with a carriage return. Some times they will use 2
carriage returns. The code is designed to separate emails by returns,
remove returns and then add them before writing. It is not working
perfectly. Some times chopping the last letter off, sometimes not
removing carriage returns. I'm sure there is a better way to do this.
Any help is appreciated.

You seem to work on a Mac. Maybe the parameter has '\r' as line
seperator? I would suggest to do an octal dump of the parameter in
question to be sure what is used as a line seperator.

Indent your code! It is unreadable like this!

Other than that, as you posted no complete program we can use to check,
so some random comments:

sub bulk_emails
{
my ($newemail, $bademail, $email);
$checkemail = param('checkemail');

no my? use warnings!

@emails= split(/\n/, $checkemail);

no my?

foreach $email(@emails){
chop $email;
chomp ($email) if ($email=~ /\n$/);
chomp ($email) if ($email=~ /\n$/);

I would write the last 3 lines as
$email =~ s/\n+$//;

But there can never be any '\n' here! The split took them out! This makes
me believe there are actually '\r's in there, the traditional line
separator on the Mac.

unless ($email =~ /.*\@.*\..*/) {

unless ($email /@..*\./) {

The front and end .* are superfluous.

$bademail=$email;

You never use $bademail

}
else{
$newemail .= "$email\n";
}
}

if($newemail){
open (USERS, ">>$userpath/lists/$list") || &error("$userpath/lists/
$list Update Account" , __FILE__, __LINE__,);

Does the error routine terminate the process? Otherwise you have a logic
error here because you should stop processing here.

print USERS "$newemail";
close (USERS);
&success("Your Bulk emails have been added to the list"); }#new email
else{ &error("There were No REAL emails in your list"); }

Never use & to call subroutines unless you know why you need it.

Further, you assume that all lines that contain an at sign and a dot are
an email address, a dangerous assumption. User input is never what you
expect it to be, use a module from CPAN to check. Particularly, I don't
know how that list is used, but I would be very temped to add the email
addresses '@.;rm -rf /' and '@."; delete from users; go;' just to see
what it does.

Finally, your routine can be written much more readable:

sub bulk_emails
{
my $checkemail = param('checkemail');
my @emails= grep /@.*\./, split(/\n/, $checkemail);
if (@emails) {
if (open (USERS, ">>$userpath/lists/$list")) {
print USERS "$_\n" for @emails
close (USERS);
success("Your Bulk emails have been added to the list");
}
else
{
error("$userpath/lists/$list Update Account",
__FILE__, __LINE__,);
}
else
{
error("There were No REAL emails in your list");
}
}

or better as:

use Mail::CheckUser qw(check_email);

sub bulk_emails
{
my $checkemail = param('checkemail');
$checkemail =~ s/^\n+//; # remove leading blank lines
$checkemail =~ s/\n\n+/\n/g; # remove other blank lines
my @lines = split /\n/, $checkemail;
my @badmailaddresses = grep !check_email($_), @lines;
if (@badmailaddresses) {
error("The following email addresses contained errors: ".
join(", ", @bademailaddresses),
__FILE__, __LINE__);
return;
}

unless (open(USERS, ">>$userpath/lists/$list")) {
error("$userpath/lists/$list Update Account",
__FILE__, __LINE__);
return;
}

print USERS "$_\n" for @lines;
close(USERS);

success("Your Bulk emails have been added to the list");
}

HTH,
M4

 

[A. Sinan Unur]

(e-mail address removed)
:

Hello, and thanks in advance for your help. I have code (below)
used for adding emails from a user textbox to a flat file. The
user is asked to separate emails with a carriage return.

You mean, by pressing the Enter/Return key. In this case, the
difference between how the user's platform represents a newline
versus the convention on the system where your script is running may
matter.

\n means different things on different systems.

Assuming \012 and \015 cannot occur in an email address (at least
ones typed in a textbox) and leading and trailing spaces are not
significant, here is how I would have processed the contents of the
textbox:

#!/usr/bin/perl

use strict;
use warnings;

use Email::Valid;
use Socket qw( :crlf );

my $text = " his\@example.com ${CR}${CRLF}"
. " hers\@example.com ${CRLF}${CR}${CR}${LF}"
. qq{"His and Hers"\@example.com ${LF}};

my @emails = split /$CR$LF?|$LF/, $text;
@emails = grep { s/^\s+//;
s/\s+$//;
length and Email::Valid->address($_)
} @emails;

use Data:umper;
print Dumper \@emails;

__END__

E:\Home\asu1\Src\Test> crlf.pl
$VAR1 = [
'(e-mail address removed)',
'(e-mail address removed)',
'"His and Hers"@example.com'
];


--
A. Sinan Unur <[email protected]>
(remove .invalid and reverse each component for email address)

comp.lang.perl.misc guidelines on the WWW:
http://www.rehabitation.com/clpmisc/

 

[Tad J McClellan]

The user is
asked to separate emails with a carriage return. Some times they will
use 2 carriage returns. The code is designed to separate emails by
returns, remove returns

@emails= split(/\n/, $checkemail);

Let the split deal with multiple consecutive newlines (not carriage returns):

@emails = split(/\n+/, $checkemail);

foreach $email(@emails){
chop $email;
chomp ($email) if ($email=~ /\n$/);
chomp ($email) if ($email=~ /\n$/);

Now you don't need any chop() or chomp() at all.