|
Re: return if defined
by Athanasius (Archbishop) on Dec 20, 2012 at 12:02 UTC
|
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,
| [reply]
[d/l]
[select] |
|
|
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;
}
| [reply]
[d/l] |
|
|
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!
| [reply] |
|
|
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!)
| [reply] |
|
|
|
|
|
|
|
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?
| [reply] |
|
|
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.
| [reply] |
Re: return if defined
by tobyink (Canon) 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'
| [reply]
[d/l] |
|
|
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);
}
| [reply]
[d/l] |
|
|
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.
| [reply]
[d/l]
[select] |
Re: return if defined
by tobyink (Canon) on Dec 20, 2012 at 12:59 UTC
|
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'
| [reply]
[d/l] |
Re: return if defined
by uncoolbob (Novice) on Dec 20, 2012 at 13:35 UTC
|
Thanks everyone - have learned a lot from your replies. | [reply] |
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;
...rest of code...
| [reply]
[d/l] |
