The horrible bug your command line Perl program probably has

| 4 Comments

Most programmers know you have to check return values from system
functions. Unless you're just starting out as a programmer, you
know that this is bad:

open( my $fh, '<', 'something.txt' );
while ( my $line = <$fh> ) {
    # do something with the input
}

If that open fails the program continues on. The call to
readline will fail, return undef as if we're at
the end of the file, and the user will be none the wiser. If you
have use warnings on, you'll get a "readline() on closed
filehandle", but that's not much help when you should be dying.

Instead, you should be opening your file like this:

my $filename = 'something.txt';
open( my $fh, '<', $filename ) or die "Can't open $filename: $!";

This way, your user gets a useful error message if something goes
wrong, but more importantly, the program doesn't continue as if
nothing is wrong, potentially doing what it should not.

GetOptions needs checking, too

Unfortunately, I see programs where otherwise-sensible programmers
ignore the return code of GetOptions.

use Getopt::Long;
GetOptions(
    'n=i' => \my $count,
);
# Do something that uses $count
print "Processing complete!\n";

There are any number of ways the user can call this program incorrectly:

$ perl foo -n
Option n requires an argument
Processing complete!

$ perl foo -n=five
Value "five" invalid for option n (number expected)
Processing complete!

$ perl foo -m=12
Unknown option: m
Processing complete!

In all three of these cases, the user made a mistake, but the program
lets it slide without a mention. The user's going to be disappointed
with the results.

The solution is simple: Always check the results of GetOptions().
The easiest way is to task && exit(1) after the call:

use Getopt::Long;
GetOptions(
    'n=i' => \my $count,
) or exit(1);

It's simple, effective, and prevents unexpected sorrow.

4 Comments

GetOptions returns a true value if it successfully parses --- shoulden't that be GetOptions() || exit(1) ?

Also, Pod::Usage provides a handy pod2usage(1) method that you can use in place of exit, which sill dump the SYNOPSIS section of your command line script's help. Useful for reminding the user what the options ought to be.

Yes, of course, thanks. I just fixed it.

And thanks for bringing up pod2usage. It's also very handy.

You should also look at autodie:

http://search.cpan.org/~pjf/autodie-2.06/

It will make unchecked open() fail in case of error.

In my own functions I often have something like

   if($error) {
      defined wantarray() or die $error;
      return $error;
   }

Which comes down to the same thing, but without external modules...

My standard Getopt::Long template looks like this:


use Getopt::Long;
GetOptions("h|help" => sub { die "\n" },
) and scalar @ARGV == X or die
"usage: $0 blah-blah-blah\n";

where "X" is the expected amount of non-flag arguments. It obviously isn't the right thing to do in every situation but for almost everything I do it works fine.

Leave a comment

Job hunting for programmers


Land the Tech Job You Love, Andy Lester's guide to job hunting for programmers and other technical professionals, is available in PDF, ePub and .mobi formats, all DRM-free, as well as good old-fashioned paper.