Ten Essential Development Practices
by Damian Conway
|
Pages: 1, 2, 3, 4, 5, 6, 7, 8
Note, however, that the contents of paragraphs are only of secondary importance here. It is the vertical gaps separating each paragraph that are critical. Without them, the readability of the code declines dramatically, even if the comments are retained:
sub addarray_internal {
my ($var_name, $needs_quotemeta) = @_;
# Cache the original...
$raw .= $var_name;
# Build meta-quoting code, if required...
my $quotemeta = $needs_quotemeta ? q{map {quotemeta $_} } : $EMPTY_STR;
# Expand elements of variable, conjoin with ORs...
my $perl5pat = qq{(??{join q{|}, $quotemeta \@{$var_name}})};
# Insert debugging code if requested...
my $type = $quotemeta ? 'literal' : 'pattern';
debug_now("Adding $var_name (as $type)");
add_debug_mesg("Trying $var_name (as $type)");
return $perl5pat;
}
8. Throw Exceptions Instead of Returning Special Values or Setting Flags
Returning a special error value on failure, or setting a special error flag,
is a very common error-handling technique. Collectively, they're the basis for
virtually all error notification from Perl's own built-in functions. For
example, the built-ins eval, exec, flock,
open, print, stat, and
system all return special values on error. Unfortunately, they
don't all use the same special value. Some of them also set a flag on failure.
Sadly, it's not always the same flag. See the perlfunc manpage for the gory
details.
Apart from the obvious consistency problems, error notification via flags and return values has another serious flaw: developers can silently ignore flags and return values, and ignoring them requires absolutely no effort on the part of the programmer. In fact, in a void context, ignoring return values is Perl's default behavior. Ignoring an error flag that has suddenly appeared in a special variable is just as easy: you simply don't bother to check the variable.
Moreover, because ignoring a return value is the void-context default, there's no syntactic marker for it. There's no way to look at a program and immediately see where a return value is deliberately being ignored, which means there's also no way to be sure that it's not being ignored accidentally.
The bottom line: regardless of the programmer's (lack of) intention, an error indicator is being ignored. That's not good programming.
Ignoring error indicators frequently causes programs to propagate errors in entirely the wrong direction. For example:
# Find and open a file by name, returning the filehandle
# or undef on failure...
sub locate_and_open {
my ($filename) = @_;
# Check acceptable directories in order...
for my $dir (@DATA_DIRS) {
my $path = "$dir/$filename";
# If file exists in an acceptable directory, open and return it...
if (-r $path) {
open my $fh, '<', $path;
return $fh;
}
}
# Fail if all possible locations tried without success...
return;
}
# Load file contents up to the first <DATA/> marker...
sub load_header_from {
my ($fh) = @_;
# Use DATA tag as end-of-"line"...
local $/ = '<DATA/>';
# Read to end-of-"line"...
return <$fh>;
}
# and later...
for my $filename (@source_files) {
my $fh = locate_and_open($filename);
my $head = load_header_from($fh);
print $head;
}
The locate_and_open() subroutine simply assumes that the call
to open works, immediately returning the filehandle
($fh), whatever the actual outcome of the open. Presumably, the
expectation is that whoever calls locate_and_open() will check
whether the return value is a valid filehandle.
Except, of course, "whoever" doesn't check. Instead of testing for failure,
the main for loop takes the failure value and immediately
propagates it "across" the block, to the rest of the statements in the loop.
That causes the call to loader_header_from() to propagate the
error value "downwards." It's in that subroutine that the attempt to treat the
failure value as a filehandle eventually kills the program:
readline() on unopened filehandle at demo.pl line 28.
Code like that--where an error is reported in an entirely different part of the program from where it actually occurred--is particularly onerous to debug.
Of course, you could argue that the fault lies squarely with whoever wrote
the loop, for using locate_and_open() without checking its return
value. In the narrowest sense, that's entirely correct--but the deeper fault
lies with whoever actually wrote locate_and_open() in the first
place, or at least, whoever assumed that the caller would always check its
return value.

