Re: return if defined

[Athanasius]

in reply to return if defined

Hello uncoolbob, and welcome to the Monastery!

How about using the Conditional Operator?

sub my_find_or_create
{
    ...
    return defined $existing_result ? $existing_result :
                                      $new_result;
}

Update: Or, better, just use Logical Defined Or:

    return $existing_result // $new_result;

Hope that helps,

[uncoolbob]

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?

[Athanasius]

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.

[Anonymous Monk]

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;
}

[uncoolbob]

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!

[Anonymous Monk]

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!)

[Anonymous Monk]

Wow, I have a split personality today.

I agree that error handling is the real bugbear of single-return. Special additional logic and/or an extra state variable are often required to make it work. It makes code less clear and sometimes even more error-prone. "Break out now" makes the most logical sense in a failure case.

But I see this example as different: there is one kind of result that could be created in one of two different ways. The if statement doesn't add any complexity to the code--you have to check for definedness somehow. Nor does it hurt clarity. Why is indenting so bad?

This is not to say that I object to your preferred solution. I think that's fine, too.