I'm overjoyed to see you put so much effort in improving your code. That's an excellent spirit, brother! ;-).
I have a couple suggestions regarding the code.
- Stay away from using globals. Store them in a nice hash inside a configuration file. There's no need to keep it all in one file, less so as globals ;).
- I know this may sound as a moot point, but I think the fewer curly brackets you have in your code the cleaner and more Perlish it looks. So, for example, start by changing all one-liner 'ifs' like so:
From this:
if ( cookie() ) { return 1; }
to this...
return 1 if cookie();
- Use true Perl, brother. For example, replace this:
if ($multi_in == 1) {
...
}
With ...
if ($multi_in) {
...
}
Or, if you expect $multi_in to be equal to a set of specific values, I'd suggest using constancs for that.
- Also, move all of your code inside convenient subroutines that do one thing at a time. Personally, I don't like a lot of clutter in the main package... try to move most of it into separate subs and it'll make the code look much cleaner and be easier to maintain.
- In addition to CGI, consider using HTML::Template to separate all your program logic from display logic. I'm not sure how advanced the module is for a newcomer, but give it a try and ask questions if things don't work out the way they should. Challenges like this are by far the best way to learn new things and improve your programming skills. For one, I used this module in virtually every Perl application I wrote thus far and don't regret the experience! ;-)
- Instead of using system mail commands to send your email messages, I suggest you use a well tested Mail::Mailer module. Here's an example:
use Mail::Mailer;
$mailer = Mail::Mailer->new();
$mailer->open({ From => $from_address,
To => $to_address,
Subject => $subject,
})
or die "Can't open: $!\n";
print $mailer $body;
$mailer->close();
The beauty of this module is that it will allow your script to be a little less platform sensitive. For example, you may still choose to use 'mail' or 'sendmail' Unix tools via this module, or you may opt instead to send your email directly via Net::SMTP (simply use smtp() method of the module).
- i'll update this node with more hits as I think of them..
UPDATE 1: added comment on
HTML::Template
UPDATE 2: Ahh, thanks UD13, I didn't notice you were already using CGI.pm. I guess, a point in case would be to pull all 'use foobar' statements at the top of your script. No use in keeping 'use ...' scattered about the script ;-)
UPDATE 3: fixed a few spelling 'bugs' thanks to
c ;)
_____________________
$"=q;grep;;$,=q"grep";for(`find . -name ".saves*~"`){s;$/;;;/(.*-(\d+)
+-.*)$/;$_=["ps -e -o pid | "," $2 | "," -v "," "]`@$_`?{print"
++ $1"}:{print"- $1"}&&`rm $1`;print"\n";}
Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
Read Where should I post X? if you're not absolutely sure you're posting in the right place.
Please read these before you post! —
Posts may use any of the Perl Monks Approved HTML tags:
- a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
| |
For: |
|
Use: |
| & | | & |
| < | | < |
| > | | > |
| [ | | [ |
| ] | | ] |
Link using PerlMonks shortcuts! What shortcuts can I use for linking?
See Writeup Formatting Tips and other pages linked from there for more info.