CODEWITHSUNDEEP
perlternary-operator
Adam S
Adam S

Reputation: 9235

Perl ternary errantly enters "else" clause?

I have the following code:

# List of tests
my $tests = [("system_test_builtins_sin", "system_test_builtins_cos", "system_test_builtins_tan")];

# Provide overrides for certain variables that may be needed because of special cases
# For example, cos must be executed 100 times and sin only 5 times.
my %testOverrides = (
    system_test_builtins_sin => {
        reps => 5,
    },
    system_test_builtins_cos => {
        reps => 100,
    },
);

my %testDefaults = (
    system_test_reps => 10,
);

# Execute a system tests
foreach my $testName (@$tests)
{
    print "Executing $testName\n";
    my $reps;

    if (exists $testOverrides{$testName}{reps})
        { $reps = $testOverrides{$testName}{reps}; }
    else
        { $reps = $testDefaults{system_test_reps}; }
    print "After long if: $reps\n";
    exists $testOverrides{$testName}{reps} ? $reps = $testOverrides{$testName}{reps} : $reps = $testDefaults{system_test_reps};
    print "After first ternary: $reps\n";
    exists $testOverrides{$testName}{reps} ? $reps = $testOverrides{$testName}{reps} : print "Override not found.\n";
    print "After second ternary: $reps\n";
}

This gives the following output:

Executing system_test_builtins_sin
After long if: 5
After first ternary: 10
After second ternary: 5
Executing system_test_builtins_cos
After long if: 100
After first ternary: 10
After second ternary: 100
Executing system_test_builtins_tan
After long if: 10
After first ternary: 10
Override not found.
After second ternary: 10

This output is most unexpected! I don't understand why the first ternary seems to always be executing the "if false" clause. It is always assigning a value of 10. I also tried changing the "false" clause to $reps = 6, and I saw that it always got the value of 6. Why does the ternary's logic depend on the content of the third (if false) clause?

Upvotes: 7

Views: 516

Answers (4)

Leif
Leif

Reputation: 1289

I would do it this way (I dont mind using ternary operators)

$reps = exists($testOverrides{$testName}{reps}) ?
    $testOverrides{$testName}{reps}             :
    $testDefaults{system_test_reps};

HTH

Upvotes: 0

David W.
David W.

Reputation: 107040

Thanks for giving us a sample of your code. Now we can tear it to pieces.

Don't use the ternary operator. It's an infection left over to originally make C programmers feel comfortable. In C, the ternary operator was used because it was originally more efficient than an if/else statement. However, compilers are pretty good about optimizing code, so that's no longer true and now it's discouraged in C and C++ programming. Programming in C is hard enough as it is without ternary operators mucking about.

The Perl compiler is also extremely efficient at optimizing your code, so you should always write for maximum clarity, so others who aren't as good as programming and get stuck maintaining your code can muddle through their job.

The problem you're having is one of operator precedence. You're assuming this:

(exists $testOverrides{$testName}{reps}) 
    ? ($reps = $testOverrides{$testName}{reps}) 
    : ($reps = $testDefaults{system_test_reps});

I would too. After all, that's what I pretty much mean. However, the assignment operator has lower precedence than the ternary operator. What's really happening is this:

(exists $testOverrides{$testName}{reps}) 
    ? ($reps = $testOverrides{$testName}{reps}) : ($reps))
    = $testDefaults{system_test_reps});

so, the final assignment is always happening to $reps.

It's much better if you use if/else:

if (exists $testOverrides{$testName}{reps}) {
   $reps = = $testOverrides{$testName}{reps};
}
else {
   $reps = $testDefaults{system_test_reps};
}

No precedence issues, easier to read, and just as efficient.

Upvotes: -2

Keith Thompson
Keith Thompson

Reputation: 263227

Here's a simpler script that illustrates the problem:

#!/usr/bin/perl

use strict;
use warnings;

my $x;

1 ? $x = 1 : $x = 0;
print "Without parentheses, \$x = $x\n";

1 ? ($x = 1) : ($x = 0);
print "With parentheses, \$x = $x\n";

It produces this output:

Without parentheses, $x = 0
With parentheses, $x = 1

I'm not sure that the relationship between assignment and ?: can be complete explained by operator precedence. (For example, I believe C and C++ can behave differently in some cases.)

Run perldoc perlop and search for "Conditional Operator", or look here; it covers this exact issue (more concisely than I did here).

In any case, I think that using an if/else statement would be clearer than using the ?: operator. Or, since both the "true" and "false" branches assign to the same variable, a better use of ?: would be to change this:

exists $testOverrides{$testName}{reps}
    ? $reps = $testOverrides{$testName}{reps}
    : $reps = $testDefaults{system_test_reps};

to this:

$reps = ( exists $testOverrides{$testName}{reps}
          ? testOverrides{$testName}{reps}
          : $testDefaults{system_test_reps} );

But again, the fact that I had to wrap the line to avoid scrolling is a good indication that an if/else would be clearer.

You might also consider using the // operator, unless you're stuck with an ancient version of Perl that doesn't support it. (It was introduced by Perl 5.10.) It's also known as the "defined-or" operator. This:

$x // $y

is equivalent to

defined($x) ? $x : $y

So you could write:

$reps = $testOverrides{$testName}{reps} // $testDefaults{system_test_reps};

This doesn't have exactly the same semantics, since it tests the expression using defined rather than exists; it will behave differently if $testOverrides{$testName}{reps} exists but has the value undef.

Upvotes: 10

mob
mob

Reputation: 118605

The -p option to B::Deparse is illuminative for problems like this:

$ perl -MO=Deparse,-p -e '$condition ? $x = $value1 : $x = $value2'
(($condition ? ($x = $value1) : $x) = $value2);

As Keith Thompson points out, it is all about the precedence. If the condition is false, the ultimate assignment is $x = $value2. If the condition is true, then the assignment is ($x = $value1) = $value2 -- either way the outcome is to assign $value2 to $x.

Upvotes: 8

Related Questions