Re^2: Module Announcement: Perl-Critic-1.01
by ikegami (Patriarch) on Jan 26, 2007 at 16:50 UTC
|
Maybe it's trying to encourage you to specify the return value explicitly, as in
return undef;
or
return ();
Update: Here's a table of the differences:
| return undef;
| return ();
| return;
|
scalar
| $sv = foo();
| undef
| undef
| undef
|
boolean
| if (foo()) { }
| false
| false
| false
|
string
| '' . foo()
| '' with warning
| '' with warning
| '' with warning
|
number
| 0 + foo()
| 0 with warning
| 0 with warning
| 0 with warning
|
list
| my @a = foo();
| ( undef )
| ( )
| ( )
|
boolean list assignment
| if (my @a = foo())
if (my ($sv) = foo())
if (() = foo())
| true
| false
| false
|
scalar list assignment
| my $sv = () = foo()
| 1
| 0
| 0
|
Note: How list assignments are treated in scalar context is what allows
while (my ($key) = each(%hash))
to work even if there's a key named 0.
| [reply] [d/l] [select] |
|
Maybe it's trying to encourage you to specify the return value explicitly...
Actually, exactly the opposite. It says you shouldn't explicitly return undef because you might actually mean return (), and that return; dwims.
| [reply] |
|
"return;" is identical to "return();"
| [reply] |
Re^2: Module Announcement: Perl-Critic-1.01
by holli (Abbot) on Jan 26, 2007 at 15:14 UTC
|
Returning an explicit undef can be a problem if the sub gets called in list context. Because then the return value will be a one element list containing an undef, which would evaluate to "true" and that is in most cases not what you want. A naked return; dwims in scalar and list contexts.
| [reply] [d/l] [select] |
|
the return value will be a one element list containing an undef, which would evaluate to "true" and that is in most cases not what you want.
Boolean list context? I'm baffled as to what case you are concerned about. As to "most cases", I quibble. Some, but not most.
| [reply] |
Re^2: Module Announcement: Perl-Critic-1.01 (just a scalar)
by tye (Sage) on Jan 26, 2007 at 20:55 UTC
|
If your function normally returns scalars, then doing return; is likely the wrong thing to do. If your function at least sometimes returns lists, then return undef; is likely the wrong thing to do. The latter is covered by other replies.
The reason that the former is wrong is because it can break code like:
my %hash= ( key1 => genValue1(), key2 => genValue2() };
# %hash= ( key1 => 'key2' ); is never what you want to get
And the problems with return undef; don't really apply to functions that only ever return a scalar because you don't write code like:
if( @array= getScalar() ) {
...
} else {
die "Couldn't get scalar";
}
So, unless Perl::Critic checks whether your function ever returns other than 1 scalar elsewhere, I consider this warning to be encouraging a bad practice (rather than just pushing a reasonable practice too hard as many people will find many of the possible warnings, surely).
| [reply] [d/l] [select] |
|
| [reply] |
|
my %hash= ( key1 => scalar lc $value );
If not, then it is probably because you know that 'lc' just returns a scalar. If you do, then there are probably a lot of people who find you silly. 'lc' isn't the only function I use that just returns scalars. Sprinkling 'scalar' all over hellenbach doesn't improve code much in my experience.
I actually consider the => to be broken for not enforcing scalar context on both sides. But that still just shifts the problem to things like function arguments.
Now, there are places where using scalar() is more appropriate. But, no, I don't consider using it on a function that just returns a scalar to be such a place. And I don't think transforming every function that might want to return an undefined scalar into "a function that returns a scalar except when it wants to return a undefined scalar in which case it returns an empty list instead and if you really wanted the undefined scalar then you should wrap the call in scalar() for that purpose" to be even close to an improvement strategy.
| [reply] [d/l] [select] |
|
|
|
|
Why make (key => scalar &functionScalar) explicit, but not return undef;? That sounds like a matter of personal preference to me.
| [reply] [d/l] [select] |
|
|
|
Re^2: Module Announcement: Perl-Critic-1.01
by Jenda (Abbot) on Jan 26, 2007 at 15:16 UTC
|
Well, whenever being explicit breaks things. return; and return undef; is NOT the same thing! It does return the same thing in a scalar context, but a very different thing in a list one. So for example
sub foo {
return undef;
}
if (@results = foo()) {
print "Helo jettero :-P\n";
}
does print the greeting. Because the function returns a list containing one element, the undef. And once you assign the list to an array and evaluate that assignment in scalar context you get the number of elements in the list and thus a true value. Probably not what you wanted, right?
| [reply] [d/l] [select] |
|
| [reply] |
|
my $pointless_reference = $obj->getPosition();
my ($x, $y) = @$pointless_reference[0,1];
or
my ($x, $y) = @{$obj->getPosition()};
instead of
my ($x, $y) = $obj->getPosition();
right? Restricting myself to returning just one value just because most languages do not allow anything more seems silly to me. Especially compared to such dirty tricks as updating the values of some variables passed by reference or something.
To me bending backwards to always return a single scalar is a code smell. A code smell sugesting that the author writes C or some other language, but definitely not Perl. Even though the code is full of sigils and there are regexps scattered around.
| [reply] [d/l] [select] |
|
|
Re^2: Module Announcement: Perl-Critic-1.01
by xdg (Monsignor) on Jan 26, 2007 at 15:22 UTC
|
I'd like to know when being explicit has ever been a problem.
It's a problem when return happens in array context -- you get a literal undef in the array. Contrived example:
use Data::Dump::Streamer;
sub filter_true {
if ( my $v = shift ) {
return $v;
}
else {
return undef;
}
}
my @values = (1, 0, 0);
my @true_values = map { filter_true($_) } @values;
Dump \@true_values
Gives:
$ARRAY1 = [
1,
( undef ) x 2
];
-xdg
Code written by xdg and posted on PerlMonks is public domain. It is provided as is with no warranties, express or implied, of any kind. Posted code may not have been tested. Use of posted code is at your own risk.
| [reply] [d/l] [select] |
Re^2: Module Announcement: Perl-Critic-1.01
by stvn (Monsignor) on Jan 26, 2007 at 16:25 UTC
|
Well, I guess it really depends on what you are being explict about :)
The other replys to this node all have valid points. However, what if I really did want to return undef? This is valid use case too.
| [reply] |
|
return undef; ## no critic
-xdg
Code written by xdg and posted on PerlMonks is public domain. It is provided as is with no warranties, express or implied, of any kind. Posted code may not have been tested. Use of posted code is at your own risk.
| [reply] [d/l] |