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

Monks, I have a situation where I'm looping through a list to send an email out. There are 9 possible email addresses, even though 99% of the time only the $email variable exists. My logs show errors every time an email is sent. It's as if this loop is trying to send to the other 8 $pscemail vars even though they are empty. 
@email = ($email,$pscemail1,$pscemail2,$pscemail3,$pscemail4,$pscemail
+5,$pscemail6,$pscemail7,$pscemail8);
    
            foreach $address (@email) {

                
                open (HTMLMAIL, "|$sendmail $address") || die "Can't o
+pen $sendmail!\n";
                print HTMLMAIL "From: $name <$adminemail>\n";
                #print HTMLMAIL "Reply-to: $fromname <$fromemail>\n";
                print HTMLMAIL "To: $address\n";
                print HTMLMAIL "Subject: $subject\n";
                print HTMLMAIL "Content-type: text/html\n";
                                Print HTMLMAIL "blah blah blah";
                                close (HTMLMAIL);
}

[download]
I'm not sure how to solve this. I assumed it simply wouldn't send if the vars were empty. This is the error I'm getting 7x: sendmail.postfix: fatal: Recipient addresses must be specified on the command line or via the -t option:

Replies are listed 'Best First'.
Re: Help with loops
by Discipulus (Canon) on May 05, 2016 at 08:22 UTC
    hello,

    if such addresses are undef they will be processed, being undef a possibility; it's up to you to check if is valid for you.

    Notice also to add use strict; use warnings; in your programs.

    As useful design pattern, put exit conditions as soon as possible: 

    foreach my $address (@email) {

       next unless $address;
       ..

       # or something more evoluted:
       unless (my_validate_mail_add($address)){
          warn "'$address' is not a valid address";
          next;
       }
       ..
}

    [download]

    L*

    There are no rules, there are no thumbs..
    Reinvent the wheel, then learn The Wheel; may be one day you reinvent one of THE WHEELS.
[reply]
[d/l]
[select]
      Thanks. Your first suggestion worked.
[reply]
Re: Help with loops
by hippo (Archbishop) on May 05, 2016 at 09:37 UTC
    @email = ($email,$pscemail1,$pscemail2,$pscemail3,$pscemail4,$pscemail5,$pscemail6,$pscemail7,$pscemail8);

    You already have a solution thanks to brother Discipulus so here's just an additional handy hint. Every time you find yourself appending a digit to the name of a scalar variable ask yourself the question, "Should I be using an array here instead?".

    The answer is almost invariably, "Yes!".

    We haven't seen your code which sets values (or not) for $pscemail1, $pscemail2, etc. but why not try just using a single @pscemail array instead and see how much neater it might make your code? HTH.

    "It doesn't work" is as useful to a developer as "I'm ill" is to a medic. Be specific.

[reply]
[d/l]
      Thanks for this. I thought I was using an array. The pscemails are set in the database. It's rare to have 8 emails, but I retrieve them by searching the database. If they are there I retrieve them with these lines: 
              $pscemail1 = $pointer->{'pscemail1'};
        $pscemail2 = $pointer->{'pscemail2'};
        $pscemail3 = $pointer->{'pscemail3'};
        $pscemail4 = $pointer->{'pscemail4'};
        $pscemail5 = $pointer->{'pscemail5'};
        $pscemail6 = $pointer->{'pscemail6'};
        $pscemail7 = $pointer->{'pscemail7'};
        $pscemail8 = $pointer->{'pscemail8'};

      [download]
      Then I put all of them into the list and send the email: 
      @email = ($email,$pscemail1,$pscemail2,$pscemail3,$pscemail4,$pscemail
+5,$pscemail6,$pscemail7,$pscemail8);
            
foreach $toemail (@email) {
Send the email
}

      [download]
      Is there a better way to do it?
[reply]
[d/l]
[select]

        Better, just be direct  @email = (  $pointer->{'pscemail1'},  $pointer->{'pscemail2'} ... );

        But thats too repetitive , too many quotes/arrows/$pointer , too much typing and/or copy/paste, so shorter  @email = @{$pointer}{ qw/ pscemail1 pscemail2 ... /});

        Thats much less repetitive and shorter, but still too repetitive, so shorter to write  @emails = map { $pointer->{'pscemail'.$_} } 1 .. 8;

        Another alternative, just to blow your mind, a tiny bit of api as language, if $pointer was more than a simple hash, if it was an object like a $row, with an emails method,

        you could use  for my $toemail ( $row->emails ){ ... }

        yeah you could eliminate @emails and just drop that map statement instead of $row->emails.... choices sure complicate stuff :D

[reply]
[d/l]
[select]
Re: Help with loops
by Anonymous Monk on May 05, 2016 at 08:37 UTC
    You know thats an insecure way to sendmail right?
[reply]
      Sorry, no. Why is it insecure?
[reply]
[reply]
[reply]

Back to Seekers of Perl Wisdom