Beefy Boxes and Bandwidth Generously Provided by pair Networks
There's more than one way to do things
 
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.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://1013504]
help
Chatterbox?
[oiskuu]: Very, very smooth btw. Now the talk is about "Your mothers pussy", and freezer implies "hungry for meat"
[Your Mother]: :P
[ambrus]: YourMother: my brother told me their cat wants to climb into the dishwasher if they leave the door open. Not when the dishwasher is started, but after it's done washing, because it's still warm and cats like warm.

How do I use this? | Other CB clients
Other Users?
Others making s'mores by the fire in the courtyard of the Monastery: (9)
As of 2017-12-18 15:24 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    What programming language do you hate the most?




















    Results (491 votes). Check out past polls.

    Notices?