Beefy Boxes and Bandwidth Generously Provided by pair Networks
The stupid question is the question not asked
 
PerlMonks  

Re^6: printing unitialized value of the 'do BLOCK'

by davido (Cardinal)
on Dec 18, 2019 at 07:40 UTC ( #11110320=note: print w/replies, xml ) Need Help??


in reply to Re^5: printing unitialized value of the 'do BLOCK'
in thread printing unitialized value of the 'do BLOCK'

I had to go look this up in Intermediate Perl because I was amazed to hear it's evangelized there. I've read the book before but it must not have stood out for me. But you're right, there it is in print. Nevertheless, I think this is not an ideal practice:

# My memory of the gist of what's in the book, a few hours after havin +g looked it up: my $thing = do { if ($cond1) { "Foo" } elsif ($cond2) { "Bar" } else { "Baz" } };

In fact it explicitly defies one of the Perl Best Practices assertions (123, I think): Always return via an explicit return. PBP assertions were almost made to be disavowed a year later; some are quite controversial, while others are useful. We may have a useful one here: This may sound totally unrelated because do only has implicit returns, but it's the example code for that assertion I'm latching onto. The code reinforcing why not to use implicit return demonstrates if statements as case-in-point for why an implicit return can be tricky. But do{...} blocks leave you only with a form of implicit return; all the more reason to code for clarity. We always expect intuitively for ternary operators to return a value. And the code above could be written with a ternary pretty easily:

my $thing = do { ($cond1) ? "Foo" : ($cond2) ? "Bar" : "Baz"; };

Now we're using a construct designed to predictably return a value, and that anyone who comes to Perl from almost any other C-inspired language could look at and immediately know what's going on. Of course in this case the do{...} block is even unnecessary:

my $thing = ($cond1) ? "Foo" : ($cond2) ? "Bar" : "Baz";

But presumably the use-case for the do{...} block would be a little less trivial; maybe something more like this:

my $thing = do { my $rn = int(rand(6))+1; ($rn == 6) ? "Excellent roll" : ($rn > 3) ? "Pretty good roll." : ($rn == 1) ? "Lousy Roll" : "Meh."; };

The difference being, we now have a need for a multi-statement construct rolling up to a single return value. Here the do is serving a purpose, but we're still using the ternary instead of the if because it is the most obvious tool for the job (so says me, but I realize that may be debatable).

I have also found that do{...} blocks have a tendency to be misunderstood by people less experienced with Perl. A year ago I found this error pervasive in our code base, contributed by more than one person, and passed through code review by several others:

sub agent { my $self = shift; return $self->{_agent} //= do { require config; my $agent_args = $config::config{'agent_args'}; require Agent; return Agent->new($agent_args); }; }

Yet another contrived example. I'm pretty sure none of the instances I found in our code were actually instantiating a UserAgent, but the problem is there regardless of what purpose it is serving. The point is that this creates a subtle bug; slightly hard to detect, and the side effect of the code actually works. I could call $self->agent->get($url) and it would work every time. The problem is that the line return Agent->new(...) returns from the do block all the way up to the caller of sub agent, bypassing the //= assignment. The $self->{_agent} element will now exist but with an undefined value. The key is there. but the assignment never happens. So every time we call $self->agent we get a new Agent object, not a cached one. Tests may all pass, because the agent is instantiated and works as it should. But (assuming we're doing this caching because it's expensive) performance suffers because the cache never populates.

So I guess what this long and rambling rant is trying to address is that do{...} is already hard enough for newcomers to the language to get right. We don't need to confuse the issue by relying on an esoteric usage of if statements. That will only serve to embolden those who say Perl is read only and that we should all be using Python.

I'm all for idiomatic Perl. I just hope we can stick to idioms that are common, clear, not confusing to people. I hate having to continually reassure people that it's not a mistake to continue using it. I appreciate code that doesn't make it harder to defend that position.


Dave

Replies are listed 'Best First'.
Re^7: printing unitialized value of the 'do BLOCK'
by rsFalse (Hermit) on Dec 23, 2019 at 08:50 UTC
    Thanks for verbose opinion and bug example!
    I will disagree that an 'if' should require an explicit return. And for me an 'if' statements are sometimes better than ternary, because an 'if' gives an opportunity to skip the 'else' branch, we can't skip it when using ternary op.
    I think that the practice suggested in Intermediate Perl is good, and it goes along with a "don't repeat yourself" principle. And 'given/when' is still an experimental. (edited)

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://11110320]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others chilling in the Monastery: (5)
As of 2020-05-25 09:32 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    If programming languages were movie genres, Perl would be:















    Results (145 votes). Check out past polls.

    Notices?