Beefy Boxes and Bandwidth Generously Provided by pair Networks
There's more than one way to do things
 
PerlMonks  

Ternary Quizical behaviour?

by bliako (Prior)
on Jul 10, 2020 at 11:35 UTC ( #11119133=perlquestion: print w/replies, xml ) Need Help??

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

I have a possibly bad habit to compact boring parts of my code like this:

if( exists($hash->{akey}) && defined($m=$hash->{akey}) && ($m==1) ){ $ +y = $m; ... } # don't trust $m here

But in this case it has unexpected results. This is the 1st part where a hash is constructed based on whether a key in another hash exists:

use strict; use warnings; use Data::Dumper; my %tests = ( 'a' => 10, 'b' => 20, ); my $m; # this seems to assign $m once and never bother to check again my %hash = ( 'b' => exists($tests{'b'}) && defined($m=$tests{'b'}) ? $m : 0, 'a' => exists($tests{'b'}) && defined($m=$tests{'a'}) ? $m : 0, ); print Dumper(\%hash); # this works as expected $m = 10; my %hash2 = ( '1' => $m++, '2' => $m++, '3' => $m++ ); print Dumper(\%hash2);
$VAR1 = { 'a' => 10, 'b' => 10 }; $VAR1 = { '1' => 10, '2' => 11, '3' => 12 };

Does anyone have an explanation? And is my habit bad?

Replies are listed 'Best First'.
Re: Ternary Quizical behaviour?
by Eily (Monsignor) on Jul 10, 2020 at 12:33 UTC

    Basically the good practice rule is to not reuse a variable in a statement where you modify it (because you might get tricked by the order of operation, or by some caching mechanism where perl only fetches the value once and uses it several times). An example of such a mistake can be found here (I remember this thread because I've been wrong with an awful lot of confidence, so disregard my answer there :P). And you could also try some variant of your second test:

    my %hash2 = ( a => ++$m, b => ++$m, c => ++$m );
    #This time you get three times the same value 0 HASH(0x30ef548) 'a' => 3 'b' => 3 'c' => 3

Re: Ternary Quizical behaviour?
by roboticus (Chancellor) on Jul 10, 2020 at 12:51 UTC

    bliako:

    I took a look at the parsed version of the code (perl -MO=Concise,-exec t.pl), and while I'm not an expert at playing with the perl opcodes, it seems to be building a list of values and then creating the hash. I've annotated it as I understand it, so I'm halfway expecting someone to chime in with a correction. So here's what I think is going on:

    # Build the entry for $m c <0> padsv[$m:1252,1255] vM/LVINTRO d <;> nextstate(main 1253 t.pl:14) v:*,&,{,x*,x&,x$,$ # Start building %hash e <0> pushmark s # First the constant "b" f <$> const[PV "b"] s # Set the value of $m to $tests{b} if it exists g <+> multideref($tests{"b"}) sK/EXISTS h <|> and(other->i) sK/1 i <+> multideref($tests{"b"}) sK j <0> padsv[$m:1252,1255] sRM* k <2> sassign sKS/2 # Now add either $m to the list (if it's defined) or 0 (if not) l <1> defined sK/1 m <|> cond_expr(other->n) lK/1 n <0> padsv[$m:1252,1255] s # add $m to the list goto o 20 <$> const[IV 0] s # or add 0 to the list # Change the value of $m to $tests{a} if it exists o <$> const[PV "a"] s p <+> multideref($tests{"a"}) sK/EXISTS q <|> and(other->r) sK/1 r <+> multideref($tests{"a"}) sK s <0> padsv[$m:1252,1255] sRM* t <2> sassign sKS/2 # If $m is defined, add it to the list, otherwise add 0 to the list u <1> defined sK/1 v <|> cond_expr(other->w) lK/1 w <0> padsv[$m:1252,1255] s goto x 1z <$> const[IV 0] s # Finish building the hash x <0> pushmark s y <0> padhv[%hash:1253,1255] lRM*/LVINTRO z <2> aassign[t5] vKS/COM_AGG

    Since the list contains references to $m instead of the current value of $m, even though $m *does* get reassigned during your expression, it doesn't matter because it only uses the final value when the hash is built from the list.

    To check my hypothesis, I changed your code a bit:

    $ cat t.pl use strict; use warnings; use Data::Dumper; my %tests = ( 'a' => 10, 'b' => 20, 'd' => 40, ); my ($m, $n); # this seems to assign $m once and never bother to check again my %hash = ( 'd' => exists($tests{'d'}) && defined($n=$tests{'d'}) ? $n : 0, 'b' => exists($tests{'b'}) && defined($m=$tests{'b'}) ? $m : 0, 'c' => exists($tests{'c'}) && defined($m=$tests{'c'}) ? $m : 0, 'a' => exists($tests{'a'}) && defined($m=$tests{'a'}) ? $m : 0, ); print Dumper(\%hash); $ perl t.pl $VAR1 = { 'c' => 0, 'b' => 10, 'd' => 40, 'a' => 10 };

    Sure enough, the element for 'd' survived since it used $n instead of $m, while the value for 'b' was clobbered because the value for 'a' also used $m.

    Edit: I need to refresh the page periodically, 'cause by the time I finally figured it out and replied, it was all said and done! ;^D

    ...roboticus

    When your only tool is a hammer, all problems look like your thumb.

      thanks, it's clear now

Re: Ternary Quizical behaviour? (updated)
by LanX (Cardinal) on Jul 10, 2020 at 12:22 UTC
    TL;dr

    but that's really what you want or just a C&P disaster? ;)

    'b' => exists($tests{'b'}) && defined($m=$tests{'b'}) ? $m : 0, 'a' => exists($tests{'b'}) && defined($m=$tests{'a'}) ? $m : 0, ^^^^^
    update

    OK now I looked closer, I think you get bitten by passing the same lvalue $m twice in the same statement.

    update

    yep, seems I'm right, if you force Perl to pass an immutable copy instead of the var it works like expected

    use strict; use warnings; use Data::Dump qw/pp dd/; my %tests = ( 'a' => 10, 'b' => 20, ); my $m; # this seems to assign $m once and never bother to check again my %hash; %hash = ( 'b' => exists($tests{'b'}) && defined($m=$tests{'b'}) ? $m : 0, 'a' => exists($tests{'b'}) && defined($m=$tests{'a'}) ? $m : 0, ); pp \%hash; %hash = ( 'b' => exists($tests{'b'}) && defined($m=$tests{'b'}) ? (0+$m) : 0, 'a' => exists($tests{'b'}) && defined($m=$tests{'a'}) ? (0+$m) : 0, ); pp \%hash; %hash = ( 'b' => exists($tests{'b'}) && defined($m=$tests{'b'}) ? "$m" : 0, 'a' => exists($tests{'b'}) && defined($m=$tests{'a'}) ? "$m" : 0, ); pp \%hash;
    { a => 10, b => 10 } { a => 10, b => 20 } { a => 10, b => 20 }

    Cheers Rolf
    (addicted to the Perl Programming Language :)
    Wikisyntax for the Monastery

Re: Ternary Quizical behaviour?
by bliako (Prior) on Jul 10, 2020 at 17:33 UTC

    OK LanX,Eily,roboticus, thanks I understand it now: The hash contents are a single statement (re: the comma) which is parsed and its elements are pushed into a stack. But $m is a reference so although its value is updated correctly, the reference pushed in the stack is always the same. So it's only the last value which will prevail. Same with ++$m. But not for $m++ which, I guess, it has to create a temporary variable to store its effect and that is what's pushed into the stack. Which is also the trick used by LanX.

    perlfan A reason I use the sequence of exists and defined is to avoid autovivification, which will appear when I use deeper levels. I know I can turn it off... (btw that's not a "sane" default!!!)

    Question: the if-statement is safe as (I guess) it evaluates each sub-conditional to a true-false and does not stack the variables? e.g. if( exists($h->{1}) && defined($m=$h->{1}) && ($m==1) && defined($m=$h->{2}) && ($m==2) ){ ... }

        I have literally never used the term alias to refer to a variable passed by reference. I don't think that's as common as you potentially think.

        Update- thank you for the replies. TIL! This is why I come here.

      I don't think that autovivification is an issue if you're consistent. But I see now.

      >Question: the if-statement is safe as (I guess) it evaluates each sub-conditional to a true-false and does not stack the variables?

      I have not tried it, but I assume that works as intended. It's just really surprising to see (in terms of the principle of lease surprise) and relies on an esoteric understanding of variable scoping (and a sharp eye). One drawback I can see is if you're relying on that honking mess being fully evaluated. Due to the short circuiting of conditions it's likely that if you treat this as a reasonable idiom that you'll run into some really freaking hard-to-debug issues caused by unexpected side effects - in otherwords, you're tying the state of a variable to external input via a potentially complex logic statement that could possibly not be evaluated fully due to said extern input. So if I were you (and I may be...<__<) I would break this habit. It's certainly not worse that autovivification, not worth the potential maintenance cost, and .. wait ...it is WAY worse than autovivification =D. HTH.

Re: Ternary Quizical behaviour?
by karlgoethebier (Abbot) on Jul 11, 2020 at 20:28 UTC

    Why is cleverness considered harmful in programming by some people?

    «Simple solutions are better for long-term maintenance. And it's not always just about language familiarity. A complex line (or lines) takes time to figure out even if you're an expert in the given language...»(From ibidem)

    Best regards, Karl

    «The Crux of the Biscuit is the Apostrophe»

    perl -MCrypt::CBC -E 'say Crypt::CBC->new(-key=>'kgb',-cipher=>"Blowfish")->decrypt_hex($ENV{KARL});'Help

Re: Ternary Quizical behaviour?
by perlfan (Priest) on Jul 10, 2020 at 12:27 UTC
    'b' => exists($tests{'b'}) && defined($m=$tests{'b'}) ? $m : 0,
    needlessly superfulous and relies on a side effect I'm a little wary of; do:

    'b' => (defined $tests{'b'}) ? $tests{'b'} : 0,
    But since the default is 0, this should mitigate the need for defined:

    b => $tests{b} // 0,

    Also, defined($m=$tests{'b'}) smells a little funny; I get what you're doing but it is unnecessary.

    Update - I am confused but the need for $m. It's getting set twice in your original code, both times based on the state of $tests{'b'}, then you set it to 10. What are you trying to do? Is this your real code or a contrived example?

      But since the default is 0, this should mitigate the need for defined
      I don't understand what you mean here. As far as I know, defined $test{'a'} ? $test{'a'} : 0 and $test{'a'} // 0 are strictly equivalent.

      Beyond that ++ for the answer, the use of // here is a clear winner :)

        I just said what you made clear, but I did so in a very awkward way. Thanks for the ++! :)
Re: Ternary Quizical behaviour?
by ikegami (Pope) on Jul 12, 2020 at 14:32 UTC

    $m puts the $m itself on the stack (not a copy), so you effectively end up assigning 'b', $m, 'a', $m (where $m is 10).

    $m++ puts the old value of $m on the stack, which must necessarily be a new variable, so you end up assigning '1', $anon1, '2', $anon2, '3', $anon3 (where $anon1 is 10, $anon2 is 11 and $anon3 is 12).


    defined is only true for keys that exists, so exists($h{x}) && defined($h{x}) can be written as defined($h{x}).

    defined($x) ? $x : 0 is equivalent to $x // 0 (except that $x is only evaluated once).

    So,

    my %hash = ( 'b' => exists($tests{'b'}) && defined($m=$tests{'b'}) ? $m : 0, 'a' => exists($tests{'a'}) && defined($m=$tests{'a'}) ? $m : 0, );
    can be written as
    my %hash = ( a => $tests{a} // 0, b => $tests{b} // 0, );

      OK thanks, got it.

Re: Ternary Quizical behaviour?
by Anonymous Monk on Jul 10, 2020 at 12:34 UTC

Log In?
Username:
Password:

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

How do I use this? | Other CB clients
Other Users?
Others contemplating the Monastery: (5)
As of 2020-09-21 18:43 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    If at first I don’t succeed, I …










    Results (126 votes). Check out past polls.

    Notices?