On 28 Mar, T Raymond wrote:

> #!/usr/bin/perl -w

Refactor with use warnings instead of -w.
-w leaks warnings to all scopes, i.e. external modules,
whereas warnings keeps itself to the local block.
See perldoc warnings therefore.

 
> print header,

What is header supposed to act on?
If it's the method declared in CGI.pm, which I'd assume,
you need to create an object by saying my $q = new CGI and
replace header with $q->header.


> start_html(-title=>'Employee Search'),
> h1('Teresa Raymond\'s Employee Search');
> 
> &search_form  unless param;

& implies passing @_ to search_form(); unneeded and very
dangerous, in most cases.


>     my $filename='employee.dat';

This is not C, so whitespaces between the variable, the assign
operator and the value is greatly recommended. See man perlstyle
therefore.


>     my $fn_weight;
>     my $ln_weight;
>     my $weight;
>     my $ot_hours;

One can argue, whether declaring those variables in several
or a single line may add up to readability. I prefer the grouped variant
in a single line, unless the variables have any familiarity at all.


>     die "cannot locate $filename\n";

Replace with "die "$filename: $!\n";
$! contains the exception message emitted by many functions
on failure.


>         for (@rows)

Are you sure, you want to have $_ evaluated instead of, say, my
$single_row? Aware of possible variable stomping?


>             if (my @matches = $fname =~ m/^$first_name/ig)
>             {

Conditions usually read CONDITION () {
}


>            if ($employees{$_}{$fname}=~m/^$first_name/ig)

Have you checked the definedness of $first_name via defined()?


> sub sub_calc_ot_pay
> {
>     my $hourly_wage = $_[0];
>     my $ot_hours = $_[1];

my ($hourly_wage, $ot_hours) = @_;


-- 
The trouble with having an open mind, of course,
is that people will insist on coming along and trying to put things in it. 
-- Terry Pratchett

-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
<http://learn.perl.org/> <http://learn.perl.org/first-response>


Reply via email to