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

Hello Monks,

I'm trying to write a simple script to help me with my network management aspects and I was wondering if it could be written more effectively or is it god enough. What do you think? 

use warnings;

chomp (my $logsIncDate = `date +"%b %d %H"`);
my $TimeStamp = `date +%Y%m%d`;
my $currentDate = `date +"%m/%d/%Y %H:%M"`;
my $logFile = "/home/log.$TimeStamp";
my $sendemail = 0;
my $sendmailPath = "/usr/lib/sendmail myemail\@bng.org\n";
my $subject  = "Subject: ";testing...
my $from     = "From: myemail\@bng.org\n";
my $to       = "To: Emyemail\@crf.org\n";
my @err = ();

##### grep through switch logs and if-found send e-mail
open SwitchLogs,$logFile or die "Could not open $logFile: $!"; 
while (<SwitchLogs>) {
    chomp;
    my $line = $_;
    if($line =~ m/$logsIncDate/ && $line =~ m/PM-4-ERR_DISABLE/){
        push(@err, "$line\n");
        $sendemail = 1;
    }
}
close SwitchLogs;

if($sendemail == 1){
    open(SENDMAIL, "|$sendmailPath") or die "Cannot open $sendmailPath
+: $!";
    print SENDMAIL $from;
    print SENDMAIL $subject;
    print SENDMAIL $to;
    print SENDMAIL "Content-type: text/plain\n\n";
    print SENDMAIL "";
    print SENDMAIL "Log messages:\n";
    print SENDMAIL "\n";
    print SENDMAIL @err;
    print SENDMAIL "\n";
    close(SENDMAIL);
}

[download]

Replies are listed 'Best First'.
Re: Make my script better
by toolic (Bishop) on Dec 11, 2013 at 20:38 UTC
    For some value of "better" (untested): 
    use warnings;
use strict;
use POSIX qw(strftime);

my $logsIncDate     = strftime('%b %d %H', localtime);
my $TimeStamp       = strftime('%Y%m%d'  , localtime);
my $logFile         = "/home/log.$TimeStamp";
my $sendmailPath    = '/usr/lib/sendmail myemail@bng.org';
my $subject         = 'Subject: Emyemail@crf.org';
my $from            = 'From: myemail@bng.org';

##### grep through switch logs and if-found send e-mail
my @err;
open SwitchLogs, '<', $logFile or die "Could not open $logFile: $!"; 
while (<SwitchLogs>) {
    push @err, $_ if /$logsIncDate/ && /PM-4-ERR_DISABLE/;
}
close SwitchLogs;

if (@err) {
    open(SENDMAIL, "|$sendmailPath") or die "Cannot open $sendmailPath
+: $!";
    print SENDMAIL <<EOF;
$from
$subject
$to
Content-type: text/plain


Log messages:

@err;

EOF
    close(SENDMAIL);
}

    [download]

    Note: you don't set $to

    Update: due to NetWallah

[reply]
[d/l]
[select]
      Shouldn't that be: 
      print SENDMAIL <<EOF;

      [download]
      ?

                   When in doubt, mumble; when in trouble, delegate; when in charge, ponder. -- James H. Boren

[reply]
[d/l]
        Of course. Like I said: untested. Updated. Thanks.
[reply]

      Thanks toolic for the tips. I like the idea of using the localtime because I do run the scripts in multiple timezones.

[reply]

      I do not have specific questions. I did try to use the terminator for the print statement before and I wasn't abe to use sendmail with it.

      Thanks for the localtime hint. I fixed my script accordingly.

[reply]

      Don't we need to define the string terminator before using it?

[reply]
        I don't understand your question. Please elaborate using code examples.
[reply]
Re: Make my script better
by PerlSufi (Friar) on Dec 11, 2013 at 21:40 UTC
    Hi, I would consider using MIME::Entity to make that first part with all the email info less ugly. For example: 
    use strict;
use warnings;
use MIME::Entity;

my $top = MIME::Entity->build(Type=>"multipart/mixed",
From => "myemail\@bng.org\n",
To  =>   "Emyemail\@crf.org\n",
Subject => "Most amazing email ever "

$top->attach (
     ...# stuff here, refer to documentation of MIME::Entity
     );
open MAIL, "|/usr/sbin/sendmail -t";
$top->print(\*MAIL);
close MAIL;

    [download]
[reply]
[d/l]

Back to Seekers of Perl Wisdom