|We don't bite newbies here... much|
Re: Some suggestions on coding style - a chance to critiqueby jarich (Curate)
|on Jun 28, 2002 at 01:15 UTC||Need Help??|
The short version:
Well I personally strongly support CGI, avoiding parameter cleansing and all the rest sounds great to me. However Juerd mentions some very good reasons against, which I can't refute. So, since you don't need all the things that CGI provides perhaps you should consider CGI::Simple which is faster, uses strict and is a very clean and nice reimplementation of CGI, by our very own tachyon :).
As everyone else says, I also recommend using more modules. Particularly something like Mail::Sendmail.
Things like this:
can be better (more clearly) written as:
And if it's appropriate, you might write it like this instead:
Either way, notice that the -> notation makes it much easier to see that $formdata is a reference to a hash of scalars. I've changed your unless eq to an if ne because in my experiences it's easier for someone to read. Trailing unlesses are great if the condition is simple such as next unless $name; but they start cluttering things if the reader has to evaluate the condition in their head and then take the complement of that.
Another style note, is that your indentation really ought to be consistant, although I hope that the inconsistancies I see here are due to cut-n-paste effects, for example "sent_email" is very difficult to read.
Hope it helps.
Update: Juerd is of course correct, the two lines are technically equivalent. I'd choose the version I've suggested because I expect that none of my students would understand the first straight off. Of course few of the students would understand the second straight off either... but it'd be easier for me to explain. ;)