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";}
| [reply] [d/l] [select] |