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

I have been using my sendmail script for a year and always wondered how I could shorten this mail part? I have many else if statements and the only difference between the mail hash parts are the Subject and Message part. If possible can you all advise how I can shorten this??
use Mail::Sendmail; #variable declarations etc... if ($old ne $curr) { my %mail = ( To => 'mail@there.com'', From => 'from@there.com', Subject => "Subject A", Message => "Subject A message here", ); $mail{smtp} = $mailserv; sendmail(%mail) || die "\nProblem! $Mail::Sendmail::error\n"; } elsif ($var eq $job) { my %mail = ( To => 'mail@there.com'', From => 'from@there.com', Subject => "Subject B", Message => "Subject B message here", ); $mail{smtp} = $mailserv; sendmail(%mail) || die "\nProblem! $Mail::Sendmail::error\n"; } else } my %mail = ( To => 'mail@there.com'', From => 'from@there.com', Subject => "Subject C", Message => "subject C message here", ); $mail{smtp} = $mailserv; sendmail(%mail) || die "\nProblem! $Mail::Sendmail::error\n"; }

Replies are listed 'Best First'.
Re: Advise on current Mail script
by Mr. Muskrat (Canon) on Jun 06, 2003 at 17:46 UTC

    Store the subject and message in variables based on the conditions prior to sending the email.

    my ($subject, $message); if ($old ne $curr) { $subject = 'Subject A'; $message = 'Subject A message here'; } elsif ($var eq $job) { $subject = 'Subject B'; $message = 'Subject B message here'; } else { $subject = 'Subject C'; $message = 'Subject C message here'; } my %mail = ( To => 'mail@there.com'', From => 'from@there.com', Subject => $subject, Message => $message, ); $mail{smtp} = $mailserv; sendmail(%mail) || die "\nProblem! $Mail::Sendmail::error\n";

Re: Advise on current Mail script
by pzbagel (Chaplain) on Jun 06, 2003 at 17:46 UTC

    One option may be to move all the %mail declarations into an array-of-hash-references or a hash-of-hash references. Then, your if-elsif-else can be converted into a CASE or SWITCH type block which sets a reference to the hash you want to send to sendmail(). This will make your script a little easier to read and make it easier to add items to the CASE/SWITCH by adding more logic and additional entries into the hash.

    OTOH, as someone once said: If it ain't baroque, don't fix it. Not to be facetious, but is this script having performance or resource problems? Security issues? Limited features? I understand the need for efficiency and I respect slick coding as much as the next monk, but unless you have a good reason, why modify something that you have been using for a year(in a production environment I assume) unless you want to expand it's functionality or correct a flaw? Just a thought.

    Later

Re: Advise on current Mail script
by blaze (Friar) on Jun 06, 2003 at 17:47 UTC
    If all you want is less code you could probably do something like
    my %mail = ( To => 'mail@there.com', From => 'from@there.com' ); if($old ne $curr){ $mail{'Subject'} = "Subject A"; $mail{'Message'} = "Subject A message here"; } elsif($var eq $job){ $mail{'Subject'} = "Subject B"; $mail{'Message'} = "Subject B message here"; } else { $mail{'Subject'} = "Subject C"; $mail{'Message'} = "subject C message here"; } $mail{smtp} = $mailserv; sendmail(%mail);

    -Robert

    Hopefully there will be a witty sig here at some point
      Thank you to all for your quick responses and suggestions!
Re: Advise on current Mail script
by hacker (Priest) on Jun 07, 2003 at 12:50 UTC
    I've been a fan of MIME::Lite and functional ternary expressions for awhile, so here's my take on it (untested, but I usually get these right on the first shot):
    use strict; use MIME::Lite; my $old = 'Foo'; my $curr = 'Foo'; my $from = 'from@there.com'; my $to = 'mail@there.com'; my $subject = 'Subject C message here'; # You could nest one more ternary in here for # '$var eq $job' $old ne $curr ? $subject = 'Subject A message here' : $subject = 'Subject B message here'; my $msg = MIME::Lite->new( From => $from, To => $to, Subject => $subject, Data => "Here's the message body you wanted", Type => 'text/plain' ); # You'd want $msg->send; though, of course print $msg->as_string;