Beefy Boxes and Bandwidth Generously Provided by pair Networks
Problems? Is your data what you think it is?
 
PerlMonks  

Re: Some suggestions on coding style - a chance to critique

by Sifmole (Chaplain)
on Jun 26, 2002 at 15:39 UTC ( #177409=note: print w/ replies, xml ) Need Help??


in reply to Some suggestions on coding style - a chance to critique

One note about style:
You might find it cleaner, and I believe it is quicker, if you group your print statements or use a "heredoc".

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 can do this as well with your  print MAIL section.

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.

&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'}; }
Why bother passing in the references? Just return the scalar values:
($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.


Comment on Re: Some suggestions on coding style - a chance to critique
Select or Download Code

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://177409]
help
Chatterbox?
and the web crawler heard nothing...

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 2014-08-01 23:05 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    Who would be the most fun to work for?















    Results (51 votes), past polls