Perl::Critic::Policy::Subroutines::ProhibitExplicitReturnUndef - Return failure with bare return
instead of return undef
.
This Policy is part of the core Perl::Critic distribution.
Returning undef
upon failure from a subroutine is pretty common.
But if the subroutine is called in list context, an explicit return
undef;
statement will return a one-element list containing
(undef)
. Now if that list is subsequently put in a boolean context
to test for failure, then it evaluates to true. But you probably
wanted it to be false.
sub read_file { my $file = shift; -f $file || return undef; #file doesn't exist! #Continue reading file... } #and later... if ( my @data = read_file($filename) ){ # if $filename doesn't exist, # @data will be (undef), # but I'll still be in here! process(@data); } else{ # This is my error handling code. # I probably want to be in here # if $filname doesn't exist. die "$filename not found"; }
The solution is to just use a bare return
statement whenever you
want to return failure. In list context, Perl will then give you an
empty list (which is false), and undef
in scalar context (which is
also false).
sub read_file { my $file = shift; -f $file || return; #DWIM! #Continue reading file... }
This Policy is not configurable except for the standard options.
You can fool this policy pretty easily by hiding undef
in a boolean
expression. But don't bother trying. In fact, using return values to
indicate failure is pretty poor technique anyway. Consider using
die
or croak
with eval
, or the Error module for a much
more robust exception-handling model. Conway has a real nice
discussion on error handling in chapter 13 of PBP.
Jeffrey Ryan Thalhammer <thaljef@cpan.org>
Copyright (c) 2005-2008 Jeffrey Ryan Thalhammer. All rights reserved.
This program is free software; you can redistribute it and/or modify it under the same terms as Perl itself. The full text of this license can be found in the LICENSE file included with this module.