in reply to Some suggestions on coding style - a chance to critique
You can do this as well with your print MAIL section.print "abc\n"; print "def\n"; print "ghi\n"; becomes print "abc\n", "def\n", "ghi\n"; or print << EOD; abc def ghi EOD
You have some pretty long parameter lists, you might want to consider named parameters.
Get rid of temporary variables:
use CGI qw(:cgi); my $url = $_[0]; my $q = CGI->new(); print $q->redirect( -url => $url ); # becomes my $q = CGI->new(); print $q->redirect( -url => $_[0] );
You use a lot of post-conditionals, and I tend to as well, but you may want to re-evaluate whether some of them make the code more difficult to read than others. A test is whether the condition or the result is "more important" and use that to determine which goes first.
Why bother passing in the references? Just return the scalar values:&get_data (\$current_date, \$remote_host, \$remote_addr, \$server_name); #... sub get_data { my ($current_date, $remote_host, $remote_addr, $server_name) = @_; #.... # get the information about who submitted the form $$remote_host = $ENV{'REMOTE_HOST'}; $$remote_addr = $ENV{'REMOTE_ADDR'}; $$server_name = $ENV{'SERVER_NAME'}; }
($current_date, $remote_host, $remote_addr, $server_name) = get_data(); sub get_data { #... # get the current date ($day, $month, $year) = (localtime)[3,4,5]; $year += 1900; return ( "$months[$month] $day, $year", $ENV{'REMOTE_HOST'}, $ENV{'REMOTE_ADDR'}, $ENV{'SERVER_NAME'}; }
And just to restate, use CGI;
Hope this is useful.
|
|---|