Beefy Boxes and Bandwidth Generously Provided by pair Networks
good chemistry is complicated,
and a little bit messy -LW

return if defined

by uncoolbob (Novice)
on Dec 20, 2012 at 11:50 UTC ( #1009725=perlquestion: print w/replies, xml ) Need Help??
uncoolbob has asked for the wisdom of the Perl Monks concerning the following question:

I was just wondering if the two lines between the comments could be made more concise and less repetitive?
sub my_find_or_create { # ... my $existing_result = $self->find_by_something(@args); return $existing_result if (defined $existing_result); # ... return $new_result; }
I thought one of these might work
return $x if (defined (my $x=foo())); # or defined (my $x = foo()) && return $x;
but it has to be like this
if (defined (my $x=foo())) { return $x }
which I guess I could live with, but it doesn't look very pretty. I think I need some kind of breakthrough, like the day I discovered ||= ;-)

Replies are listed 'Best First'.
Re: return if defined
by Athanasius (Chancellor) on Dec 20, 2012 at 12:02 UTC

      Thanks for the welcome! I tried to answer another question while I was here.

      I think the ? : operator solution would be equally verbose and wouldn't the code be creating $new_result (a DBIC row object) even if we didn't need to return it?

        wouldn't the code be creating $new_result ... even if we didn't need to return it?

        The conditional operator works like if-then-else, so when the condition is true the clause following the : is not evaluated. Likewise for logical defined or: if the first expression is defined, the second expression (the one following //) is not evaluated.

        But in either case $new_result would need to be computed within the relevant clause; otherwise, as Anonymous Monk says, the short-circuiting is removed. If the code (not shown) used to populate $new_result is too long to make this practical, then prefer the solutions proposed by Anonymous Monk or tobyink.

        Athanasius <°(((><contra mundum Iustus alius egestas vitae, eros Piratica,

      I am a fan of the defined-or operator, but in this case both of your solutions remove the short-circuiting. uncoolbob's original sub did not generate the new result if the existing result was already defined, and that good feature should be preserved.

      How about something like this? Using one variable called $result simplifies the code and allows a single return statement (I am not dogmatic about having only one return statement, but I think it is nicer in a case like this).

      sub my_find_or_create { my $result = $self->find_by_something(@args); unless (defined $result) { # create result here } return $result; }
        Hi Anon, thank you - I do sometimes use what you propose and I really like the easy to follow logic/flow. I can probably do this in my current case - it's a bit more complex than my example but the code is definitely in need of some tidying!

        I view your sample code the very thing wrong with the "single return" principle. My dogma is: return as early as you can if it avoids extra indentation.

        (One of the usual offenders is a lengthy if {} block with the main logic, coupled with a short else {} block whose only purpose is to return some error value. Just reverse the blocks' order and the main logic no longer needs an extra indent level!)

Re: return if defined
by tobyink (Abbot) on Dec 20, 2012 at 12:39 UTC
    sub my_find_or_create { # ... return $self->find_by_something(@args) // do { # ... }; }
    perl -E'sub Monkey::do{say$_,for@_,do{($monkey=[caller(0)]->[3])=~s{::}{ }and$monkey}}"Monkey say"->Monkey::do'

      Clever, but maybe a little too clever.

      This isn't the clearest construct in the world to begin with, and it especially has the potential to be confusing if the code within the do block is long. In that case, the last statement of the block is implicitly returned by the block and used as the return value, but it is a long way from the return statement itself.

      You could do the exact same thing a lot more clearly just by using a subroutine.

      sub my_find_or_create { # ... return $self->find_by_something(@args) // $self->create_new(@args); }
        Clever, but maybe a little too clever. This isn't the clearest construct in the world ...

        But I thought the whole point of the OP was to be too clever by half, and clarity be damned.

        IMHO, the pair of statements
            my $existing_result = $self->find_by_something(@args);
            return $existing_result if (defined $existing_result);
        is perfectly clear and sufficiently unclever to help me avoid future foot trauma.

Re: return if defined
by uncoolbob (Novice) on Dec 20, 2012 at 13:35 UTC
    Thanks everyone - have learned a lot from your replies.
Re: return if defined
by tobyink (Abbot) on Dec 20, 2012 at 12:59 UTC

    Another possibility...

    use PerlX::Perform; use return::thence; sub my_find_or_create { # ... perform { return::thence $_ } wherever $self->find_by_something(@arg +s); # ... }
    perl -E'sub Monkey::do{say$_,for@_,do{($monkey=[caller(0)]->[3])=~s{::}{ }and$monkey}}"Monkey say"->Monkey::do'
Re: return if defined
by Anonymous Monk on Dec 20, 2012 at 12:48 UTC

    Personally, I find this the easiest to read:

    my $x = $self->find_by_something(@args); defined $x and return $x; of code...

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://1009725]
Approved by bingos
and all is quiet...

How do I use this? | Other CB clients
Other Users?
Others chilling in the Monastery: (3)
As of 2018-04-21 02:15 GMT
Find Nodes?
    Voting Booth?