Chop vs Chomp

Discussion in 'Perl Misc' started by zoomcart.com, Apr 1, 2008.

  1. zoomcart.com

    zoomcart.com Guest

    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");
    }
     
    zoomcart.com, Apr 1, 2008
    #1
    1. Advertising

  2. zoomcart.com

    J. Gleixner Guest

    zoomcart.com wrote:
    > 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()"
     
    J. Gleixner, Apr 1, 2008
    #2
    1. Advertising

  3. On Tue, 01 Apr 2008 08:14:15 -0700, zoomcart.com wrote:

    > 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
     
    Martijn Lievaart, Apr 1, 2008
    #3
  4. "zoomcart.com" <> wrote in
    news:29fda9bd-9c9f-4db5-9273-

    :

    > 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::Dumper;
    print Dumper \@emails;

    __END__

    E:\Home\asu1\Src\Test> crlf.pl
    $VAR1 = [
    '',
    '',
    '"His and Hers"@example.com'
    ];


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

    comp.lang.perl.misc guidelines on the WWW:
    http://www.rehabitation.com/clpmisc/
     
    A. Sinan Unur, Apr 2, 2008
    #4
  5. zoomcart.com <> wrote:

    > 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.


    --
    Tad McClellan
    email: perl -le "print scalar reverse qq/moc.noitatibaher\100cmdat/"
     
    Tad J McClellan, Apr 2, 2008
    #5
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. gusmeister

    Inverse of 'chop @array'

    gusmeister, Mar 6, 2004, in forum: Perl
    Replies:
    1
    Views:
    688
    J├╝rgen Exner
    Mar 6, 2004
  2. Guest
    Replies:
    7
    Views:
    680
    Walter Wang [MSFT]
    Jun 28, 2006
  3. Johnathan Smith

    chop and chop!

    Johnathan Smith, Jan 8, 2008, in forum: Ruby
    Replies:
    2
    Views:
    139
    darren kirby
    Jan 8, 2008
  4. yusufm

    print chop; VS chop; print;

    yusufm, Mar 9, 2006, in forum: Perl Misc
    Replies:
    2
    Views:
    119
    Tad McClellan
    Mar 9, 2006
  5. martin
    Replies:
    3
    Views:
    186
    Joe Smith
    Apr 15, 2006
Loading...

Share This Page