PHP short if behavior

Question

Possible Duplicate:
How can I understand nested ?: operators in PHP?

Why does this:

  $object->customer->phone =
          ! empty( $object->customer->address->phone_fixed ) ?
                    $object->customer->address->phone_fixed :
          ! empty( $object->customer->address->phone_mobile ) ?
                    $object->customer->address->phone_mobile :
          ! empty( $object->customer->address->phone_business ) ?
                    $object->customer->address->phone_business : '';

returns $object->customer->address->phone_business, even if $object->customer->address->phone_fixed is set and not empty?

Thank you.

The short if syntax is **left associative**. – Mihai Stancu Jul 06 '12 at 14:53 — Mihai Stancu, Jul 06 '12 at 14:53

score 5 · Accepted Answer · answered Jul 06 '12 at 14:52

5

It has to do with the associativity of the ternary operator. I would at a minimum add parens to your code; but honestly... more than one ternary in a single statement is a modern faux-pas . Interesting link

answered Jul 06 '12 at 14:52

Chris Trahey

18,202
1
42
55

+1 Nice, *interesting link*. Forgot about [PHP Sadness](http://www.phpsadness.com/sad/30) – Jason McCreary Jul 06 '12 at 15:00
Thanks, I refactored with if/elseif. – cili Jul 06 '12 at 15:03

score 3 · Answer 2 · answered Jul 06 '12 at 14:51

3

Either your data is not what you think (debug to see) or your logic is wrong.

While this short if (ternary operator) is nice for simple, inline assignment, I'd encourage you to refactor this into if/else blocks for clarity.

I imagine once doing so, your problem will become clear. And in turn prove the point about writing clean code.

answered Jul 06 '12 at 14:51

Jason McCreary

71,546
23
135
174

-1 The short if syntax is **left associative**. – Mihai Stancu Jul 06 '12 at 14:54
4

Never said otherwise. Why don't you post an answer instead of down voting others. Either way, that would fall under a misuse of code and therefore covered under *your logic is wrong* :) – Jason McCreary Jul 06 '12 at 14:56
Because this is a duplicate question I've already answered somewhere else. I don't want to copy-paste my answer because that's not constructive either. SO wants one question to be answered only once on the site. – Mihai Stancu Jul 06 '12 at 15:06
Then you should have left a comment with a link to that particular topic instead of simply bandwagoning answers in this topic with a downvote comment for perfectly legit and appropriate answers. Its not polite to dismiss other people's effort to offer guidance and advice on question related stuff. A clear +1 from me on this answer simply for fairness sake. – Jul 06 '12 at 15:20
@Mihai Stancu, understood on the duplication. I've closed many questions as such. However, I don't believe in down voting. Especially when an answer is neither incorrect nor misleading. – Jason McCreary Jul 06 '12 at 17:23
@holodoc I did indeed leave a comment on the original question with the link to the duplicate. Stack overflow lets us choose how we behave and I choose to down-vote answers that tackle side-issues even if the answers are correct in relation to the side-issues. I consider that my behaviour helps Stack Overflow sort good->better->best answers and show the next 100 visitors the most relevant one(s) first. – Mihai Stancu Jul 06 '12 at 21:26
@Mihai Stancu, you are indeed entitled to behave as you wish on SO. However, given my reputation and the *support* of my answer and original comment I clearly know how SO works. Furthermore, it's not a *side-issue* to provide best-practice suggestion in a *teach a man to fish* style answer. Sometimes the *best answer* is not the one that *solves* the problem. Hence the voting system. Maybe you should consider that before you down vote answers. Your multiple comments and suggestion to close the question are more than enough. Down voting brings negativity. That behavior doesn't help. – Jason McCreary Jul 06 '12 at 21:54
With your SO experience this shouldn't create negativity for you. I upvoted @ctrahey for relevance and downvoted you for having less relevance. As you've probably experienced in many places this is not a personal thing it's a matter of (justifiable) opinion. – Mihai Stancu Jul 06 '12 at 22:06
I'm not taking anything personal. I am disagreeing with your behavior of up voting a single answer and down voting the rest as *less relevant*. Such behavior is undeniably negative and obviously discouraged on SO - by the fact that down voting takes way from your reputation as well. – Jason McCreary Jul 06 '12 at 22:20
The fact that it takes away from my reputation is not a reflection of it being discouraged by SO, it's a reflection of the consciouns responsibility I must devote to the fact that I don't agree with something. – Mihai Stancu Jul 08 '12 at 13:17
Also I didn't downvote all the other answers, only the ones that were not on the right track (offering correct but collateral support). And I upvoted @ctrahey for being the only answer on the right track. – Mihai Stancu Jul 08 '12 at 13:18

score 1 · Answer 3 · answered Jul 06 '12 at 14:59

This is why using the ternary operator for such complex assignments is not a good idea. Whenever I nest ternary operations though, I always enclose each operation in parenthesis to be sure.

I would recommend using multiple if/else or switch statements.

if (!empty($object->customer->address->phone_fixed)
    $object->customer->phone = $object->customer->address->phone_fixed;
elseif (!empty($object->customer->address->phone_mobile)
    $object->customer->phone = $object->customer->address->phone_mobile;
elseif (!empty($object->customer->address->phone_business)
    $object->customer->phone = $object->customer->address->phone_business;

score 0 · Answer 4 · answered Jul 06 '12 at 14:50

0

Most likely the variables don't contain what you think they do, use var_dump and do some debugging.

I didn't even know you could do elseif's using the short behaviour. If I was you I would just rewrite that chunk of code to make it more readable / reliable / predictable etc.

In my opinion simply writing several if's would look much nicer and to anyone reading the code, they would immediately understand it.

// Sorry for not answering the question.

answered Jul 06 '12 at 14:50

472084

17,666
10
63
81

-1 The short if syntax is **left associative**. – Mihai Stancu Jul 06 '12 at 14:53
My point is that you are making no effort to actually answer the question. You are simply offering advice as to how to approach and avoid the situation. Which should be a comment not an answer. – Mihai Stancu Jul 06 '12 at 15:03
I meant your point regarding left associativity, never mind. – 472084 Jul 06 '12 at 15:09

PHP short if behavior

4 Answers4