Re: Returning undef: The point I would like Damian to reconsider
by shmem (Chancellor) on Jun 22, 2007 at 12:43 UTC
|
Don't change subroutine results based on list or scalar context.
Why? I have useful subroutines that produce data and behave different in void, scalar and list context:
$thingy -> foo (1,2); # results printed to current filehandl
+e
$string = $thingy -> foo (1,2); # results returned as a single string
@lines = $thingy -> foo (1,2); # results returned as a bunch of lines
and I don't see that as a bad practice. Works like expected and is documented. The alternative would be to shove the result explicitly through three other subs:
$thingy -> foo (1,2) -> to_output_channel;
$string = $thingy -> foo (1,2) -> as_string;
@lines = $thingy -> foo (1,2) -> as_list;
which gains me what exactly?
Perl is context aware, even builtins do change their result depending on context, eg localtime.
Why should I avoid perl's dwimmery? Should I avoid localtime and roll my own?
update: I have PBP on my shelf as well, and my biggest critic to it is about its title. It should have been named "Damian Conway's Coding Style", or "Perl Pitfalls" subtitle "and how to avoid them", or even "Code Perl Robustly" subtitle "without knowing all of perl".
My biggest critic upon Perl::Critic is that it enforces PBP's bad title as a standard, and where it could help you it doesn't, and will never: it can't tell whether you know what you are doing or have fallen into a trap, since it can't read your mind.
update 2: Personally I think that the rule "Use a bare return to return failure" cannot be a rule, since its text omits context. It's just bad worded and should be:
To return failure, use a bare return in list context, undef in scalar or void context. Set $@ to something meaningful.
Perl::Critic should handle the rule that way.
--shmem
_($_=" "x(1<<5)."?\n".q·/)Oo. G°\ /
/\_¯/(q /
---------------------------- \__(m.====·.(_("always off the crowd"))."·
");sub _{s./.($e="'Itrs `mnsgdq Gdbj O`qkdq")=~y/"-y/#-z/;$e.e && print}
| [reply] [Watch: Dir/Any] [d/l] [select] |
|
I ++'d your post but I disagree about your comments on the title of PBP. The book is full of well thought out recommendations that have been vetted by several members of the Perl community so I really do not see a problem with the name. Yes there will be some cargo cult type followers and yes, incompetent management will want to use it as a style guide. But really, given the choice between PBP and an in-house standard cobbled together by said incompetent management which would you prefer!? Overall I think the Perl community has needed something like PBP for a long time and the benefits of its existence far outweigh the negatives (including the name).
| [reply] [Watch: Dir/Any] |
|
I disagree about your comments on the title of PBP.
...
the benefits of its existence far outweigh the negatives (including the name).
Don't you see a contradiction here? So far you have defended the book, and I agree. You did not, though, explain why you disagree with me saying the title is bad - you just say it doesn't matter.
Well, for me, it matters.
But really, given the choice between PBP and an in-house standard cobbled together by said incompetent management which would you prefer!?
What choice is that? Given the choice between a burger from <insert-junk-food-company-here> and your own fried shoe soles without salad and no onion, what would you prefer?
I would see this most excellent book from Damian Comway as a style guide or vade mecum to gain perl insight and develop a good inhouse coding standard, rather than having the book as a whole imposed as such, without reading it, and merely because of its title.
Which means: if anybody, including the PHB, after reading it gets enlightened and shouts "hey, these are really jolly well best perl practices!" and goes to introduce a subset or all as standard, that's perfectly fine.
It is not ok if someone comes and says "here, have Perl Best Practices. It's standard now." - "Why?" - "Because they are. Can't read the title?"
--shmem
_($_=" "x(1<<5)."?\n".q·/)Oo. G°\ /
/\_¯/(q /
---------------------------- \__(m.====·.(_("always off the crowd"))."·
");sub _{s./.($e="'Itrs `mnsgdq Gdbj O`qkdq")=~y/"-y/#-z/;$e.e && print}
| [reply] [Watch: Dir/Any] |
|
|
and I don't see that as a bad practice. Works like expected and is documented. The alternative would be to shove the result explicitly through three other subs:
$thingy -> foo (1,2) -> to_output_channel;
$string = $thingy -> foo (1,2) -> as_string;
@lines = $thingy -> foo (1,2) -> as_list;
which gains me what exactly?
It gains you
- explicit clarity of intent
- 3 parameterizable output methods
- 3 methods which can be re-used or overloaded as needed
- The ability to call all 3 output methods, none or any combination simply by mixing/matching method calls.
Should I avoid localtime and roll my own?
Well, no. Try Date::localtime or Time::Local or anything in the DateTime hierarchy.
Carter's compass: I know I'm on the right track when by deleting something, I'm adding functionality
| [reply] [Watch: Dir/Any] [d/l] |
|
1. explicit clarity of intent
I have that on the LHS.
2. 3 parameterizable output methods
3. 3 methods which can be re-used or overloaded as needed
4. The ability to call all 3 output methods, none or any combination simply by mixing/matching method calls.
No call for those, since that doesn't gain me anything.
You missed my point. Context awareness is a key concept of Perl. Deprecating context awareness with regard to subroutine return is tantamount to crippling the language.
Try Date::localtime
No such module... ;-)
--shmem
_($_=" "x(1<<5)."?\n".q·/)Oo. G°\ /
/\_¯/(q /
---------------------------- \__(m.====·.(_("always off the crowd"))."·
");sub _{s./.($e="'Itrs `mnsgdq Gdbj O`qkdq")=~y/"-y/#-z/;$e.e && print}
| [reply] [Watch: Dir/Any] |
Re: Returning undef: The point I would like Damian to reconsider
by jettero (Monsignor) on Jun 22, 2007 at 10:43 UTC
|
I posted a similar sentiment back in January of this year. My comments aren't totally worth mentioning, but Tye and Chromatic have a rather fascinating argument about it that's worth reading [596717]. There are about thirty other nice posts under there too, many of which I haven't even seen before.
| [reply] [Watch: Dir/Any] |
Re: Returning undef: The point I would like Damian to reconsider
by merlyn (Sage) on Jun 22, 2007 at 21:06 UTC
|
Don't change subroutine results based on list or scalar context.
If I were trying to follow that advice, what should I do if a subroutine that returns a list in a list context gets called instead in a scalar or void context? "die"?
Just trying to think this through.
Or did you actually mean "if your subroutine naturally returns a scalar, be sure to return that same scalar in a list context unless otherwise documented" or something convoluted like that?
And the timing for this couldn't be better... I'm just now putting the finishing touches on the Perl Best Practices full-day course that I'm presenting at YAPC::NA next week, and have had my own thoughts on many issues such as these during the writing of the course.
| [reply] [Watch: Dir/Any] |
|
In the past, when faced with this question, I've usually gone on the principle
that if a function normally/ever gets called in list context, then it should
always return a "list", even in scalar context, i.e. if called in scalar
context return the last value in its return expression. This, imho, is the
minimally surprising behavior in most scenarios, and is also easiest to code as
you don't have to code for sensitivity to calling context. OTOH, obviously there
may be times when you'd rather have some other behavior; the only other one you
should "reasonably" implement is having it act like an array, i.e. return the
return list length when called in scalar context. But even that would warrant
documentation including several exclamation points. Any other behaviors
should, I think, be limited strictly to internal function calls, not in a public
interface.
A word spoken in Mind will reach its own level, in the objective world, by its own weight
| [reply] [Watch: Dir/Any] |
|
then it should always return a "list", even in scalar context, i.e. if called in scalar context return the last value in its return expression.
Ugh, that's not possible. You cannot return a list in a scalar context. Ever.
You can type:
return (99, 100, 101);
But that's not "returning a list in a scalar context", even if called in a scalar context. It's returning a scalar in a scalar context, by evaluating that comma-expression in a scalar context, getting the result 101.
I know you know that, but sloppy language here gets us running around in circles.
See my On Scalar Context for samples of many things that "listy" things do in a scalar context. Based on that, I don't see how "last thing" is any more intuitive than "first thing" or "number of elements" (the three main candidates).
| [reply] [Watch: Dir/Any] [d/l] |
|
|
|
|
Don't change subroutine results based on list or scalar context.
If I were trying to follow that advice, what should I do if a subroutine that returns a list in a list context gets called instead in a scalar or void context? "die"?
Coercing a strictly list returning function into returning a scalar would be wrong and could be treated as an error of the caller if detected.
But you caught me there, the subroutine would still need to be context-aware to detect that situation.
The impact on the subroutine code could be kept at bay, though, if we tucked that piece of logic away, say, in a listonly convenience function.
Is something like that already on CPAN?
Hmmmm...
| [reply] [Watch: Dir/Any] |
|
use Contextual::Return;
use Carp qw(croak);
...
sub foo {
...
return
LIST { some listy expression here }
SCALAR { croak "please invoke me in a list context" }
;
}
| [reply] [Watch: Dir/Any] [d/l] |
Re: Returning undef: The point I would like Damian to reconsider
by CountZero (Bishop) on Jun 23, 2007 at 08:55 UTC
|
I think "The One Rule" is that you must clearly document the return values of your methods or subroutines and expect the users to read the docs.
CountZero A program should be light and agile, its subroutines connected like a string of pearls. The spirit and intent of the program should be retained throughout. There should be neither too little or too much, neither needless loops nor useless variables, neither lack of structure nor overwhelming rigidity." - The Tao of Programming, 4.1 - Geoffrey James
| [reply] [Watch: Dir/Any] |
|
Hmm... would an all-encompassing "One Rule" be called the "One Rule to Ring Them All"...?
:-)
| [reply] [Watch: Dir/Any] |
Re: Returning undef: The point I would like Damian to reconsider
by whereiskurt (Friar) on Jun 22, 2007 at 14:41 UTC
|
I love perlmonks because it constantly reaffirms that there's always more than one way to do it. When you have a question, there's usually more than one 'right' answer. The question is: when I call some encapsulated piece of code, how will I know whether it succeeded or failed? Was there an error and what should I do?
The reason why I like the PBP 'rule' is because it's an easy to remember answer to a question that should be considered on every function call. Defensive programming means checking return values. The concept of what to return on failure is like a mini-protocol. "Use a bare return to return failure" is such an easy protocol....
For all the folks who think it's stupid, and hate the idea of typing 'return 1;', make up your own mini-protocol and a call it Perl's SecondBest Practices. LOL :-)
"Don't return undef when you mean an empty list" -- this is some serious wisdom, too. I'm really glad you said that. :-) I think it's worth mentioning that PBP also recommends using Contextual::Return to deal with that exact 'complication'.
IMO The Best Practice here is a combination of using "return;" anywhere something goes wrong and at the bottom of your subs using a block like this:
use Contextual::Return;
sub items {
## Something failed!
if ($failure) return; #TODO: Consider croaking!
## Being explicit!
return (
LIST { qq(Item1 Item2) }
HASHREF { {Item1 => 'big', Item2 => 'small'}}
SCALAR { 2 }
);
}
I think knowledge sharing and promoting 'the right way' is what will allow the Perl Community to continue to flourish. I think Damian was Noble in pursuing this ideal.
Kurt
Updated: Added points about Contextual::Return | [reply] [Watch: Dir/Any] [d/l] |
Re: Returning undef: The point I would like Damian to reconsider
by chromatic (Archbishop) on Jun 22, 2007 at 17:37 UTC
|
An empty return statement would be a mistake there, precisely because the function might be called in list context. In Perl, list context is sometimes inconvenient to avoid, for example if an expression happens to be used as a subroutine argument. We should not expect our undef function result to vanish and be replaced by the next argument in such a situation, I think.
The only time where this would harm the code I tend to write is if I had a semi-predicate problem in a function that returned a list. (The only reason I don't run into the problem where hash declarations impose list context is that I don't do much CGI programming, where I might call $q->param( 'foo' ) in a hash assignment.)
| [reply] [Watch: Dir/Any] [d/l] |
Re: Returning undef: The point I would like Damian to reconsider
by Raster Burn (Beadle) on Jun 22, 2007 at 16:05 UTC
|
I think the truly "best" practice would be to use an exception class hierarchy and throw exceptions upon errors. I <3 Exception::Class | [reply] [Watch: Dir/Any] |
|
I think the truly "best" practice would be to use an exception class hierarchy and throw exceptions upon errors. I <3 Exception::Class
Agreed, but that was not the point of this guideline.
To be fair to TheDamian, he dedicated a whole chapter to error handling, where among other things he discusses exception hierarchies in depth.
Damian's point in this guideline was, one should use an empty return statement where returning undef could lead to unexpected results, while my main point was that he failed to properly distinguish that exotic case from good uses of returning undef, seeing that now some programmers think it should be avoided altogether.
Personally, I prefer to use empty return statements only for void context, i.e. subroutines or methods that don't offer a return value.
If I found myself catering for context-aware return value polymorphism I would not mind to use a more explicit construct.
| [reply] [Watch: Dir/Any] [d/l] [select] |