Beefy Boxes and Bandwidth Generously Provided by pair Networks
more useful options
 
PerlMonks  

Re: Seeking guidance for more idiomatic way of (re)writing this script.

by topher (Scribe)
on Jan 16, 2013 at 05:49 UTC ( #1013504=note: print w/ replies, xml ) Need Help??


in reply to Seeking guidance for more idiomatic way of (re)writing this script.

  • You've got 2 places where you send e-mail. If you do something twice, it's a good candidate for splitting out into a function. I'd probably write a function called "send_mail()" or something, and put all of your e-mail work there. You can call it with an option to tell it what type of e-mail it should send.

  • As someone else mentioned, if most of your code is in an else {} block, that's a red flag that you might want to refactor. Since the initial if {} block is basically just checking to make sure you opened the file, I'd probably do something like:

    open(my $fh , "<" , $array_ip_list) or send_mail("ip_list_error"), die "IP List File - nas_array_ip_li +st.txt not found";

    Where send_mail() is the function mentioned earlier. The send_mail() function will check its argument, see "ip_list_error" and know what to do. This way, you either successfully open the file, or you send your e-mail and exit.

  • Definitely pull out any "global" settings and put them together at the front. That'll make it significantly easier to update later. As mentioned, a hash can be useful for this. You can use a single hash if you don't have too many things, or a couple of them if it makes logical sense.

  • Since you're deleting dmcheck.txt at the end and only using it as a temporary holding place for your e-mail, you'd be better off to use File::Temp. It's a standard module since 5.6.1, and gives you a safer way of using a temp file.

  • Minor style point: in my experience, most Perl programmers don't use parentheses around the conditional in a postfix if statement. For example:

    next if /foo/;

    If you're doing anything much more complicated than that, you'll want to use parens as appropriate of course.

  • Other than that, you're off to a good start already, and there's a bunch of other good suggestions here, too.


Comment on Re: Seeking guidance for more idiomatic way of (re)writing this script.
Select or Download Code

Log In?
Username:
Password:

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

How do I use this? | Other CB clients
Other Users?
Others musing on the Monastery: (22)
As of 2015-07-02 18:25 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    The top three priorities of my open tasks are (in descending order of likelihood to be worked on) ...









    Results (44 votes), past polls