Perl Best Practices
by Damian Conway

The unconfirmed error reports are from readers.  They have not yet been 
approved or disproved by the author or editor and represent solely the
opinion of the reader.

Here's a key to the markup:
        [page-number]: serious technical mistake
        {page-number}: minor technical mistake
        <page-number>: important language/formatting problem
        (page-number): language change or minor formatting problem
        ?page-number?: reader question or request for clarification

This page was updated April 7, 2008.


UNCONFIRMED  errors and comments from readers: 


(xxi) top of page, 2nd line down && 14th line down;
... extrapolate from those particulars to excellent general advice.
?should be?
... extrapolate from those particulars excellent general advice.
(remove 'to')

-and-

... and his valiant defence of "unless".
should be
... and his valiant defense of "unless".
(spelling of defense)

[15] Last paragraph of Section 2.6., Operators.;
The final paragraph of Section 2.6., Operators, states, "Named unary operators should be treated 
like builtins, and spaced from their operands appropriately"; however, the first example given 
fails to account for operator precedence:

my $tan_theta = sin $theta / cos $theta;

This is clearly intended to calculate the tangent of an angle theta by dividing the sine of 
theta by the cosine of theta:

sin(theta) / cos(theta)

However, because the precedence of the division operator "/" is higher than the precedence of 
the named unary operators "sin" and "cos," the result is actually calculated as follows:

sin( theta / cos(theta) )

Here is one possible correct form of the example:

my $tan_theta = sin($theta) / cos($theta)

Here is a shell transcript that proves that the code snippet in the book produces the wrong 
result, and that the form immediately above produces the correct result:

$ perl -e '$theta = 1; print(sin $theta / cos $theta, "\n");'
0.961050079349205
$ perl -e '$theta = 1; print(sin($theta) / cos($theta), "\n");'
1.5574077246549
$ perl -e 'use Math::Trig; $theta = 1; print(tan($theta), "\n");'
1.5574077246549

I characterized this as "serious technical mistake" because this chapter is supposed to 
demonstrate best practices for code layout, yet the author's own formatting advice from Section 
2.4, Don't use unnecessary parentheses for builtins and "honorary" builtins, appears to have led 
to the construction of an erroneous expression in Section 2.6.

Perhaps the omission of parentheses on named unary operators in expressions should not be held 
up as a best practice. At the very least, some additional discussion of this pitfall of Perl 
operator precedence might be in order.


(40) top code block;

while (my $month = prompt -menu => $MONTH_NAMES) {
for my $book ( @catalog ) {
print "$ISBN_for{$book} $title_of{$book}: $sales_from[$month]\n";
}
}

According to section '2.4. Builtins', only builtins do not need "unnecessary parentheses".

Prompt is not a builtin and therefore should have parentheses in order to "visually 
distinguish between subroutine calls and calls to builtins".


{42}  second code listing (in boldface);
As noted in the comments of the first (non-boldface) code example on the page, the
_ref variables should use dereferencing arrows to access values.  Thus, the two
lines:

   my $gap    = $opts_ref{cols} - length $text;
   my $left   = $opts_ref{centred} ? int($gap/2) : 0;

should read instead:

   my $gap    = $opts_ref->{cols} - length $text;
   my $left   = $opts_ref->{centred} ? int($gap/2) : 0;


{44}  Second code sample (middle of page);
Shouldn't $tax_form be $tax_form_ref, especially this close to the Reference
Variables recommendation?


{100} 4th line up from bottom of page (last line before code snippet);
Function name is incorrect - it should be odd() (the function from the previous code snippet), 
but the text refers to even() which DNE:

"... then a better solution is usually to "inline" the call to even(), replacing ..."

-Should Be-

"... then a better solution is usually to "inline" the call to odd(), replacing ..."


[164]  Code example near bottom of page;
Sample code in book describing the Sort::Maker module has a usage/syntax error and
will not compile.

Please see my usenet post regarding this issue (in GoogleGroups:
http://tinyurl.com/j4hzg) and also the reply in this thread from Uri Guttman (author
of the Sort::Maker module) confirming the error.


{195} Example code;
In the discussion of prototypes, the example code includes a subroutine called "clip_to_range" 
that takes $min, $max, and @data as parameters. Based on that information, I expected the code 
to return only those data elements that are between $min and $max.

Inspection of the code, however, revealed that through the use of List::Util's min and max 
routines data elements that were less than $min were changed to $min and those greater than $max 
were changed to $max. The code in question is map { max( $min, min( $max, $_ ) ) } @data.

For example, given $min = 1, $max = 3, and @data = ( 0 .. 4 ) I expected the subroutine to 
return ( 1, 2, 3 ). It actually returns ( 1, 1, 2, 3, 3 ).

If this is the intended behavior of the subroutine, I suggest renaming it (as the use of "clip" 
implies cutting and removing) and/or adding a comment that indicates that is the intended 
behavior. If the code itself is in error, one simple fix might be something like grep { ($min <= 
$_) && ($_ <= $max) } @data.


(198-199)  bottom of 198, top of 199;
Note: I found this in the PDF sample chapter from the book's web site; I have not
verified whether it is in a full copy of the book itself.  I did not find it in the
errata.

There is a code snippet that starts on the bottom of page 198 and continues on the
top of page 199.  The code consists of four if statements with two comment lines in
the body of each statement.  The second comment line is incorrect in each of the
statements.  They begin:

  # so if the block is ...

whereas they should be:

  # so the if block is ...

That is, "if" and "the" are transposed.

This code snippet parallels a code snippet on page 197.  The code on that page does
not have this problem.


{220}  last code example (and first code example p 221 as well);
Replace
    -onechar
with
    -one_char
    
        
[242]  1st paragraph;
Regexp::Autoflags does not exist in CPAN.


(292) Code paragraph;
I believe the code should not be shown in bold, as it represents an example of something that is 
wrong.


{304} Last word;
The last word, 'it', should have a footnote explaining that the unlinking process proposed does 
not work under MS Windows, and that IO::InSitu should be used instead.


[327]  File::System->list_files;
When I run the script I get this: File::System->list_files

There is no mention of using File::System in this portion of the book. I installed
File::System and added use File::System at the top of the script and it validates,
but when I run it it says:

Can't locate object method "list_files" via package "File::System" at
/Users/bradrice/lib/perl5/File/Hierarchy.pm line 26.


(399) 3rd last line;
Violation of "don't use barewords" rule in:

prompt -line, '>'

which should be changed to:

prompt '-line', '>'

There's another error at the top of page 40 where:

prompt -menu => $MONTH_NAMES

could be changed to:

prompt '-menu' => $MONTH_NAMES

but at least the "fat comma" forces the left hand side to be stringified.


{410}  Perl6::Export::Attrs issues;
One of the most important paradigm shifts in this book is getting users to use
Class::Std module to build new perl modules (which ROCKS by the way). However,
Perl6::Export::Attrs is not compatible with Class::Std. This shoudl be pointed out in
the book. Or the modules need to be fixed.


{466-467}  {19.14}  Ch. Miscellanea, Sect. Enbugging
(http://safari.oreilly.com/0596001738/perlbp-CHP-19-SECT-14);
The code refactoring is mildly flawed because the original

    $target_name = $1 and last

is not behaving the same way as the refactored

    $target_name = $name;
    last CANDIDATE;

for value "0", i.e. the candidate string is "Name: 0; "