On Tue, 31 Aug 2004, Gavin Henry wrote:

# Globals
my $rdiff = '/usr/bin/rdiff-backup';
my $localdir = '/home/ghenry/perl';
my $userhost = "[EMAIL PROTECTED]";
my $remotedir = '/home/slim_jim/perl';
my $args = "$localdir $userhost\:\:$remotedir";
my $to = "[EMAIL PROTECTED]";
my $from = "[EMAIL PROTECTED]";
my $time = localtime;
my $datestamp =  strftime "%d.%m.%y.%T", localtime;

It's nitpicking, but I like lining repetitive lines like these up so that the differences stand out better, like so:


    my $rdiff     = '/usr/bin/rdiff-backup';
    my $localdir  = '/home/ghenry/perl';
    my $userhost  = "[EMAIL PROTECTED]";
    my $remotedir = '/home/slim_jim/perl';
    my $args      = "$localdir $userhost\:\:$remotedir";
    my $to        = "[EMAIL PROTECTED]";
    my $from      = "[EMAIL PROTECTED]";
    my $time      = localtime;
    my $datestamp =  strftime "%d.%m.%y.%T", localtime;

Also, it seems to me like a good habit to minimize putting in literals when you can help it -- typos are all too easy to make. Therefore,

    my $to        = "[EMAIL PROTECTED]";
    my $from      = $to;

If $from needs to be something else later, you can change it. (This is kind of a thin example, but I'm trying to point out a broader idea.)

# Suretec Message
print 
"\n----------------------------------------------------------------------------\n";
print "Brought to you by Suretec Systems Ltd.\n";
print "----------------------------------------------------------------------------\n";

Heredocs are popular for statements like this, but I prefer this idiom:

  print qq[
    ------------------------------------------------------
    Brought to you by Suretec Systems Ltd.
    ------------------------------------------------------

    ------------------------------------------------------
    Initialising remote backup synchronsation on $time.B
    ------------------------------------------------------
  ];

Less typing; more readable. IMO.

# Send e-mail with a few details for success and failures
# Success
if ($backup == 0) {
my %mails = ( To => "$to",
From => "$from",
Subject => "Remote backup complete from $ENV{HOSTNAME} on $time",
Message => "The remote backup has been completed on $ENV{HOSTNAME} on $time
with the command:\n\n $rdiff $args\n" );

Again, whitespace makes stuff like this a lot easier to read:

  my %mails = (
    To      => "$to",
    From    => "$from",
    Subject => "Remote backup complete from $ENV{HOSTNAME} on $time",
    Message => "The remote backup has been completed on $ENV{HOSTNAME}"
               . " on $time with the command:\n\n $rdiff $args\n"
  );

sendmail(%mails);

A lot of people like avoiding temp variables like this, and would rather just combine the `my %mails` declaration into the `sendmail(...)` call. But whatever, that's a judgement call -- it may make the code a hair faster, but probably not enough to care about; the bigger issue is whether the temp variable is more readable, but that's a judgement call.


# Create a success logfile
open LOG, ">>$datestamp-suretec-backup-success.log"
 or die "Cannot create logfile: $!";
print LOG "Remote backup completed on $time, with the command:\n\n$rdiff
$args\n\nAn e-mail has been sent.\n";

Again, I think the qq{...} syntax can make this kind of thing cleaner:

 print LOG qq{Remote backup completed on $time, with the command:

    $rdiff  $args

An e-mail has been sent.
};

# Did it work?
die "Backup exited funny: $?" unless $backup == 0;

A lot of people like to `die` immediately when an error comes up, rather than waiting until the end of the script like this. The common idiom is:


   some_risky_thing(...) or die "You sunk my battleship! $!";

or

   die "You sunk my battleship! $!" unless some_risky_thing(...);

which, again, comes down to which seems more readable. This can depend a lot on the particular example -- in this case, I think the first version looks cleaner, because it puts the common case first, while the second puts the exceptional case first and so looks backwards. In other cases, that might be fine though.



--
Chris Devers      [EMAIL PROTECTED]
http://devers.homeip.net:8080/blog/

np: 'Movin' Right Along (lo-fi midi version)'
     by The Muppets
     from 'The Muppet Movie Soundtrack'

--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
<http://learn.perl.org/> <http://learn.perl.org/first-response>




Reply via email to