Re: creating a variable in an if statement
by dragonchild (Archbishop) on Jan 22, 2008 at 20:52 UTC
|
but the question is - can I do this all in a single expression?
(I know you know this, but others may not and so it should be emphasized.)
The emphasis in maintainable code (and all code spends 80% of its time in maintenance mode, so good code is maintainable code) should be on readability. An expression is readable relative to the rest of the code if the number of units I have to parse does not go up significantly from the surrounding code. I would posit that a difference of 3 or more renders an expression hard to read relative to the surrounding code. Given that most lines of Perl (in my experience) run between 3 and 5 units, anything that has more than 6-8 units is going to be very hard to read.
Your snippet contains, by my count, 9 units. I would argue that it should be written as so:
{
my @f = some_code();
@f = some_other_code() unless @f == 1;
if ( @f == 1 ) {
# ....
}
}
That could be improved quite a bit if what some_code() returns is clarified. If it returns @f == 0 or @f == 1, then other solutions can be provided. But, if it can legally return @f == 2, then the above is really the only good solution. Of course, some_code() and some_other_code() already sound like sucky functions and I have no idea what they do. I'm basing this solely on what you seem to have to do in order to work with them in a sane fashion.
My criteria for good software:
- Does it work?
- Can someone else come in, make a change, and be reasonably certain no bugs were introduced?
| [reply] [d/l] |
Re: creating a variable in an if statement
by ikegami (Patriarch) on Jan 22, 2008 at 19:50 UTC
|
Variables are only available (by name) in the statement following the one in which they are declared. It allows the following to work.
my $var;
...
{
my $var = $var;
...
}
So the key is to not refer to the new var by name or to put multiple statements in the if expression.
The only think that comes to mind is
if (
sub {
our @f; local *f = $_[0];
(@f = some_code()) == 1 || (@f = some_other_code()) == 1
}->(\my @f)
) {
#...
}
Yuck.
This isn't a general purpose solution, but it works here:
if (my @f = do {
my @f;
((@f = some_code()) == 1 || (@f = some_other_code()) == 1) ? @f : (
+)
}) {
#...
}
Yuck.
Update: Added a missing pair of parens to the last snippet.
| [reply] [d/l] [select] |
Re: creating a variable in an if statement
by kyle (Abbot) on Jan 22, 2008 at 20:47 UTC
|
I say: just put the my @f outside, as you've done in your last code block. All the other solutions I've seen so far are much uglier. Here's my contribution to the ugly solution set (with Test::More test framework, so you can see the one tye has up right now—but since fixed—doesn't actually work).
Update: Here's my one line:
use List::Util qw( first );
if ( my @f = @{(first { @{$_=[$_->()]}==1 } $code1, $code2)[0]||[]} )
Also, I added the anonymous reply (it fails several tests).
Update again: Changed tye's solution to what I think was originally intended. | [reply] [d/l] [select] |
Re: creating a variable in an if statement
by Anonymous Monk on Jan 22, 2008 at 21:59 UTC
|
When you're writing code, keep it simple and clear. Nothing else really matters.
If you want to confine a certain variable to a limited scope, it's fine to use "{}" braces to create a block, put "my" variable declarations inside that block and go-to-town. The variables will be confined to the static scope of that block. But do that only when that's clear.
“Obfuscations” are for idle-amusement only!
| [reply] |
|
I agree.
...it's fine to use "{}" braces to create a block...
You could then consider giving the block a name i.e. create a sub. You can then
- move the block out of your main code and, with a good descriptive name, make the code more readable
- use the "return early, return often" idiom which can often help simplify and clarify your intentions
- write it so that all arguments are passed to it so testing it is much easier
- re-use it
| [reply] |
Re: creating a variable in an if statement ("do"s)
by tye (Sage) on Jan 22, 2008 at 19:54 UTC
|
if( do { my @f; 1 == ( @f= one() ) || 1 == ( @f= two() ) } ) {
but that still leaves the body of your 'if' statement on separate lines. So you probably want to change your semicolons to commas, add parens where needed, maybe a few more 'do's so you can use a statement-modifier 'if' instead. The number of lines is really getting out of hand here. All that whitespace makes the multiple layers of nested parens and braces not nearly as interesting to stare at.
Update: Oh, the @f doesn't survive outside of the do { }.
if( my @f= do { my @g; 1 == ( @g= one() ) || 1 == ( @g= two() ) } ) {
| [reply] [d/l] [select] |
|
| [reply] [d/l] [select] |
|
Somebody who makes rampant updates to so many of their nodes after posting might want to check for updates (actually noted as such) from others before replying or, horror, wait a few minutes.1 :)
1 I think it is unlikely that you started replying before my update was posted as I refreshed to see my update, then refreshed for other reasons and saw your node had been updated (with no notice) and you hadn't replied yet. So after my update you made your own update and then replied to my node based on a version from before your last update. But I give up trying to update/reply fast enough for you. I'll check back after a few hours and assume your node content won't expand several fold w/o comment (yes, I've seen that happen with your nodes more than once, as I've mentioned to you before) from that point and thus it might be safe to reply/update if any response is warranted. (:
| [reply] |
|
|
|
Re: creating a variable in an if statement
by Anonymous Monk on Jan 22, 2008 at 20:59 UTC
|
perl -wMstrict -e
"sub S1 { return int rand 2 ? ('one 1') : () }
sub S2 { return int rand 2 ? ('one 2') : ('bad', 'news') }
for (0 .. shift) {
if (1 == (my @f = S1 || S2)) { print @f }
else { print 'not one' }
print qq(\n);
}" 10
one 1
one 1
one 1
not one
one 2
not one
one 2
one 1
not one
one 1
not one
| [reply] [d/l] |
|
if (1 == (my @f = S1 || S2)) {
scalar context is forced upon the return of S1, but not on that of S2:
perl -le '@g = qw(a b c); @h = qw(d e); @f = @g || @h; print @f'
3
perl -le '@g = qw( ); @h = qw(d e); @f = @g || @h; print @f'
de
What if S1 returns more than one element?
--shmem
_($_=" "x(1<<5)."?\n".q·/)Oo. G°\ /
/\_¯/(q /
---------------------------- \__(m.====·.(_("always off the crowd"))."·
");sub _{s./.($e="'Itrs `mnsgdq Gdbj O`qkdq")=~y/"-y/#-z/;$e.e && print}
| [reply] [d/l] [select] |
|
sub S1 { return qw( a b ); }
sub S2 { return qw( c ); }
S2 would get called and the result would be @f=qw(c). But with yours, S2 doesn't get called and the result is @f=qw(b).
| [reply] [d/l] [select] |
Re: creating a variable in an if statement
by Cefu (Beadle) on Jan 23, 2008 at 15:43 UTC
|
Serveral people have already pointed out why the solution in your last snippet (the my before your if statement) is the best. You should just accept this rather than try for that "more perfect" one liner.
I just wanted to quickly address the confusion about the error message regarding masking the variable. You might think that there will never be a second my declaration for the same variable (thus masking the first declaration) because only one side or the other of the || will be true. If the first condition is true then you're fine; only one declaration is executed. But what does the parser actually do when the first condition is false?
The first my declaration happens, the some_code() assignment happens and only then is it found to be false. Now the parser moves on to the other side of the || and hits a second my declaration. It doesn't somehow forget or throw away what happened before the || just because it didn't turn out to be true.
| [reply] [d/l] [select] |
|
Actually, perl has a three pass 'compiler', for want of a better term, followed by a tree-threaded interpreter. The order of execution is not known at the time the declarations are compiled into the code tree. If you have multiple declaration with a statement there is no 'execution order' all the declarations get compiled into the code tree initially.
perlguts says 'bottom up' I'm not sure what this means. I think in this case it means that the 'if' is built from the inside out and the compiler, upon building out one branch of the conditional, can't see down the other branch to spot the other declaration, but that's just speculation on my part.
s//----->\t/;$~="JAPH";s//\r<$~~/;{s|~$~-|-~$~|||s
|-$~~|$~~-|||s,<$~~,<~$~,,s,~$~>,$~~>,,
$|=1,select$,,$,,$,,1e-1;print;redo}
| [reply] [d/l] |