CODEWITHSUNDEEP
perloperator-precedenceconditional-operator
Mose
Mose

Reputation: 563

Perl ternary conditional operator

I'm trying to write more efficient code in my scripts and have been implementing ternary conditional operators on occasion. I can't understand why I am getting an additional result when using a ternary conditional operator in a loop:

#!/usr/bin/perl

use strict;
use warnings;

my @array = ('Serial = "123"', 'Serial = "456"', 'Serial = "789"');
my ($test1,$test2);
foreach my $a (@array){
        !$test1 ? $test1 = $a  : $test1 .= " AND " . $a;
}
foreach my $b (@array){
        if (!$test2) {
                $test2 = $b
        } else {
                $test2 .= " AND " . $b;
        }
}
print "Test1: $test1\n";
print "Test2: $test2\n";

Output:

~/bin/test.pl
Test1: Serial = "123" AND Serial = "123" AND Serial = "456" AND Serial = "789"
Test2: Serial = "123" AND Serial = "456" AND Serial = "789"

Test1 output has an additional "Serial = "123", What am I doing wrong?

Upvotes: 4

Views: 4816

Answers (5)

ChrisWue
ChrisWue

Reputation: 19020

Assignment has lower precendence than ?. This

!$test1 ? $test1 = $a  : $test1 .= " AND " . $a;

is equivalent to this:

(!$test1 ? $test1 = $a  : $test1) .= " AND " . $a;

So first $test1 will become Serial = "123" and then AND Serial = "123" gets appended immediately after.

Try this:

!$test1 ? ($test1 = $a)  : ($test1 .= " AND " . $a);

A better solution would be this:

$test1 = !$test1 ? $a  : $test1 . " AND " . $a;

Using the ternary operator for side effects can get quite messy and I'd recommend to avoid it.

Edit

As noted by MuIsTooShort join(' AND ', array) would be the most concise and readable solution in your case.

Upvotes: 7

run
run

Reputation: 1186

In your loop 

my ($test1,$test2);
foreach my $a (@array){
        !$test1 ? $test1 = $a  : $test1 .= " AND " . $a;
}

1st time $test1 will be :Serial = "123" AND Serial = "123"

2nd time : Serial ="123" AND Serial = "123" AND Serial = "456"

3rd:Serial "123" AND Serial = "123" AND Serial = "456" AND Serial = "789"

4ht:Test1: Serial = "123" AND Serial = "123" AND Serial = "456" AND Serial = "789"

Upvotes: 0

ikegami
ikegami

Reputation: 385645

First, you have a precedence problem.

!$test1 ? $test1 = $a : $test1 .= " AND " . $a;

means

( !$test1 ? $test1 = $a : $test1 ) .= " AND " . $a;

It can be solved with parens.

my $test1;
for (@array) {
   !$test1 ? ($test1 = $a) : ($test1 .= " AND " . $a);
}

But that's not readable. You're obviously going in the wrong direction! There are two tasks being performed, and you are trying to jam them into one. Simply separating them makes the code much more readable.

my $test1;
for (@array) {
   $test1 .= ' AND ' if $test1;
   $test1 .= $_;
}

But we're not there yet. Let me introduce you to join.

my $test1 = join(' AND ', @array);

So much better!

Finally, it sure looks like you are building an SQL statement. If so, your question is moot since you should USE PLACEHOLDERS TO TRANSFER DATA TO A DATABASE. Search the DBI documentation for that word.

Upvotes: 11

Zaid
Zaid

Reputation: 37136

The ternary operator is great for situations when Z = X OR Y.

You're not really doing that here. What's happening is that you're building a string from an array. That's what the join function is all about.

my $test = join ' AND ', @array;

Now that is more efficient coding.

Upvotes: 2

Alex
Alex

Reputation: 5893

You're hitting a precedence problem. You can see how Perl has interpreted (i.e. understood) an expression using Deparse

perl -MO=Deparse,-p -e "!$test1 ? $test1 = $a  : $test1 .= q{ AND } . $a;"

the -p tells Deparse to put in lots of brackets so you can see what's up:

(((!$test1) ? ($test1 = $a) : $test1) .= (' AND ' . $a));

so here you can see that the line of code doesn't mean what you intended.

It's fairly bad style to use ?: like this, and won't make things faster.

Upvotes: 2

Related Questions