Beefy Boxes and Bandwidth Generously Provided by pair Networks
Do you know where your variables are?
 
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 browsing the Monastery: (9)
As of 2014-12-18 05:10 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    Is guessing a good strategy for surviving in the IT business?





    Results (42 votes), past polls