Beefy Boxes and Bandwidth Generously Provided by pair Networks
There's more than one way to do things
 
PerlMonks  

Re: Can I clean this up??

by frankus (Priest)
on Jul 24, 2002 at 10:50 UTC ( #184761=note: print w/replies, xml ) Need Help??


in reply to Can I clean this up??

First off consider what the if statement is doing:
open(FIL,">>$myfil") || die "No open for ct tot: $! \n"; if($ct == 1){ print FIL "\n$ct is down at this time.\n"; } elsif($ct >= 2){ print FIL "\n$ct are not available at this time.\n"; } system("mailx -s 'Mail header' myemail < $myfil"); close(FIL);

Secondly consider using a pipe instead of writing to a file:
open(MAIL,"|mailx blah) || die "No mail stuff: $! \n"; if($ct == 1){ print MAIL "\n$ct is down at this time.\n"; } elsif($ct >= 2){ print MAIL "\n$ct are not available at this time.\n"; } close(MAIL);
Try some common Perl tricks:
# Removed brackets, replaced || with or due to higher presedence and r +eadability # used single quotes on uninterpolated strings. open MAIL,'|mailx blah' or die "No mail stuff: $!\n"; # Stuck the if statements at the end so it's more natural. print MAIL "\n$ct is down at this time.\n" if $ct == 1; print MAIL "\n$ct are not available at this time.\n" if $ct >= 2; close(MAIL);

What? more elegant?.....um, it's arguable.
open(MAIL,"|mailx blah) || die "No mail stuff: $! \n"; SWITCH: { $ct >= 2 && { print MAIL "\n$ct are not available at this time.\n +"; last SWITCH }; $ct == 1 && { print MAIL "\n$ct is down at this time.\n"; last SW +ITCH }; } close(MAIL);

--

Brother Frankus.

¤

Replies are listed 'Best First'.
Re: Re: Can I clean this up??
by PhiRatE (Monk) on Jul 24, 2002 at 21:59 UTC
    Nice work. Lets add a bit of holistic consideration. What exactly is he trying to achieve? well, very simply it's a plural translation within english. In those instances where more than one system is down, he wishes to maintain a correct english sentence.

    We thus minimise our code duplication like this (perl):

    open(MAIL,"|mailx blah") || die "No mail stuff: $! \n"; if ($ct>0) { print MAIL "\n$ct ".($ct>1 ? "are not available" : "is down")." at + this time.\n"; } close(MAIL);

    But of course the greatest optimisation is always achieved by considering the whole problem, not just the perl:

    open(MAIL,"|mailx blah") || die "No mail stuff: $! \n"; print MAIL "\nSystems unavailable at this time: $ct\n" if ($ct>0) close(MAIL);

    Silly I know, but to the point: Good optimisation looks at the whole problem.

Re: Re: Can I clean this up??
by Anonymous Monk on Jul 24, 2002 at 12:03 UTC
    Wow, thanks for all the suggestions! Now I can make it look better.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://184761]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others surveying the Monastery: (6)
As of 2016-10-01 20:11 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    How many different varieties (color, size, etc) of socks do you have in your sock drawer?






    Results (7 votes). Check out past polls.