Beefy Boxes and Bandwidth Generously Provided by pair Networks
P is for Practical
 
PerlMonks  

Useless use of string in return statement

by Bod (Chaplain)
on Apr 12, 2021 at 23:06 UTC ( #11131157=perlquestion: print w/replies, xml ) Need Help??

Bod has asked for the wisdom of the Perl Monks concerning the following question:

Is there something strange about the way return treats conditions?

I have this and it doesn't work as expected...

sub get_ids { my ($self, %attrs) = @_; # Do stuff... my %result; # $result{'message'} = ''; if ($self->{'error'}) { $result{'status'} = 'error'; $result{'message'} = $self->{'error'}; } else { $result{'status'} = 'success'; $result{'api-key'} = $self->{'api-public'}; $result{'session'} = $intent_id; } return encode_json(\%result) if lc($attrs{'format'}) eq 'json'; return $result{'message'} or "$result{'api-key'}:$result{'session' +}"; # <- line 229 return "SOMETHING"; }
If it is called as get_ids( 'format' => 'json' ); it works fine but asking it to return a text string returns undef and warns Useless use of string in void context at line 229. The way I think it should work is that if $result{'message'} evaluates as true, that will get returned but if it evaluates as false then "$result{'api-key'}:$result{'session'}" wil be returned instead.

Can you explain why this is not behaving as expected?

As an aside, in searching for an answer I found this post -> Useless use of string in void context
There it is suggested that Perl reports the wrong line number for this warning so it is quite possible that I'm actually looking in the wrong place!

Replies are listed 'Best First'.
Re: Useless use of string in return statement
by tybalt89 (Prior) on Apr 12, 2021 at 23:29 UTC

    Because

    return $result{'message'} or "$result{'api-key'}:$result{'session'}";   # <- line 229

    is really

    (return $result{'message'}) or "$result{'api-key'}:$result{'session'}";   # <- line 229

    try || instead of or

Re: Useless use of string in return statement
by GrandFather (Saint) on Apr 12, 2021 at 23:32 UTC

    I get:

    Possible precedence issue with control flow operator at noname.pl line + 23. Useless use of string in void context at noname.pl line 23.

    for the return $result{'message'} ... line, which makes sense. Remember or is low precedence so the compiler sees:

    (return $result{'message'}) or "$result{'api-key'}:$result{'session'}" +;

    High precedence || would fix the problem

    Update: note that I'm using Perl v5.32.0.

    Optimising for fewest key strokes only makes sense transmitting to Pluto or beyond
      I can never remember the rules of precedence, so I just remove the relevance of such considerations by using brackets to make my intentions clear:
      C:\>perl -MO=Deparse,-p -e "return $x or $y" ((return $x) or $y); -e syntax OK C:\>perl -MO=Deparse,-p -e "return ($x or $y)" (return ($x || $y)); -e syntax OK
      Cheers,
      Rob
Re: Useless use of string in return statement
by hippo (Chancellor) on Apr 13, 2021 at 08:42 UTC

    It has been my habit of long standing to use the word forms of the logical operators (and, or and not) purely for control flow and to use the symbolic forms (&&, || and !) for comparing values within an expression. This makes precedence bugs of the kind you have reported here much less frequent.

    HTH.


    🦛

      Ditto! Strongly agree!! :)

      Note that Perl Best Practices, chapter 6 (Control Structures), "Use block if, not postfix if" argues that postfix-if does not scale as well as block-if, and is harder to comprehend (except in simple cases). I agree, especially with the scaling argument. Always using block-if has made code reviews more enjoyable for me over many years because there are fewer changed lines of code to review whenever you just add an extra statement to a block-if (compared to more violently restructuring the code from postfix-if to block-if).

      In chapter 4 (Values and Expressions), "Don't mix high- and low-precedence booleans", Conway recommends using the low precedence and and or operators for flow of control, for example:

      open(my $fh, '<', $file) or die "error opening '$file': $!";
      while reserving && and || for logical expressions (not flow of control) for example:
      if ($x > 5 && $y < 10) ...
      Following this simple rule over the years has made the code easier to understand at a glance, at least for me.

        Use block if, not postfix if

        Are you suggesting postfix if should never be used?

        I agree that adding an extra conditional statement requires more work and more consideration but there are cases, like my conditional return statement, where it is rather unlikely that any extra statements would want to be added to the conditional execution.

Re: Useless use of string in return statement
by kcott (Bishop) on Apr 13, 2021 at 05:50 UTC

    G'day Bod,

    When you encounter this sort of issue, you'll often find using B::Deparse, to see how Perl interprets your code, will help.

    $ perl -MO=Deparse,-p -e 'return $x or $y' ((return $x) or $y); -e syntax OK $ perl -MO=Deparse,-p -e 'return $x || $y' (return ($x || $y)); -e syntax OK

    — Ken

      Thanks Ken
      I've seen B::Deparse mentioned here and there and keep meaning to find out what it does and why I might want it. I guess you've just answered that for me!

Re: Useless use of string in return statement
by AnomalousMonk (Bishop) on Apr 13, 2021 at 03:38 UTC

    FWIW, note also that the
        return "SOMETHING";
    statement in the OPed code is unreachable.


    Give a man a fish:  <%-{-{-{-<

      FWIW, note also that the
      return "SOMETHING";
      statement in the OPed code is unreachable.

      Yes - I should have pointed out the reason for that line in the question...
      It was added as part of trying to work out what was actually happening. Having a definite return ensured that I was not getting a return value from the last evaluated expression. I did try to eliminate some of the possibilities before turning to the Monastery.

Re: Useless use of string in return statement
by davido (Cardinal) on Apr 13, 2021 at 16:31 UTC

    I love this question because it identifies a problem that is a common pitfall. I've even discovered it in the past creeping into code at work.

    Look at this code:

    sub foo { return $bar or $baz }

    Pass that through B::Deparse:

    perl -MO=Deparse -e 'sub foo {return $bar or $baz}' sub foo { $baz unless return $bar; }

    The code return $bar has to be executed before its value can be calculated for the unless condition. But return exits the subroutine, and leaves "unless" hanging. Control passes out of the sub. Game over for the unless statement. It never gets an opportunity to regain control of flow, so $baz could never be hit.

    We could run very similar code through Deparse using the || operator:

    perl -MO=Deparse -e 'sub foo {return $bar || $baz}' sub foo { return $bar || $baz; }

    Here we find none of the surprising behavior, the expression $bar || $baz has to get evaluated first, then the return happens.

    The return could be coerced to work "as you wish" by using parenthesis, too:

    sub foo { return ($bar or $baz); }

    We can pass that through B::Deparse as well:

    perl -MO=Deparse -e 'sub foo {return ($bar or $baz)}' sub foo { return $bar || $baz; }

    Look at that: It parsed down to the || operator.

    We had another one pop up in some code a year or so ago. It's not exactly the same, but in a way similar:

    sub foo { my $filename = shift; my $content = do { local $/; open my $fh, '<', $filename or die $!; return scalar(<$fh>); } $content = s/abc/def/g; return $content; }

    Spot the problem? You shouldn't use return in a do block unless you actually intend to return out of the surrounding subroutine. The writer of that code knew that do blocks return something to their caller. So they used a return to make it explicit (rather than falling back to the last thing that got evaluated in the do block). But that's incorrect; the return returned out of the foo subroutine, leaving the assignment to $content hanging, and the remainder of the subroutine unreachable. In many of the cases I spotted of this, it wasn't apparent what was happening because the caller of foo() still got a return value that seemed reasonable. I think in the actual case where I saw it, the context was poor-man's caching:

    sub get_foo { my $self = shift; return $self->{'foo'} ||= do { # expensive operation; return $result; } }

    Here the biggest clue there was a problem was that the call to that accessor took longer than it should on any call after the first time it was called. That's rather hard to test for, though a test that breaks encapsulation and looks at the object's guts would have noticed.


    Dave

Re: Useless use of string in return statement
by Anonymous Monk on Apr 13, 2021 at 00:47 UTC
    Try immediate-if:  return $result{'message'} ? $result{'message'} : "$result{'api-key'}:$result{'session'+}";

      Every single line of code untenable…

      syntax error at - line 1, near "+}" Execution of - aborted due to compilation errors.
        Not to mention that return $a ? $a : $b; can always* be written as return $a || $b; ... which is exactly what tybalt already said.

        (*unless $a is tied.)

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://11131157]
Approved by huck
Front-paged by haukex
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others perusing the Monastery: (4)
As of 2021-05-10 07:32 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    Perl 7 will be out ...





    Results (104 votes). Check out past polls.

    Notices?