Comparison Operator "IF" in PHP

Question

Possible Duplicate:
nested php ternary trouble: ternary output != if - else

Why this work:

if($db->f('item_bonus') > 0)
    $bonus = $db->f('item_bonus').' (i)';
elseif($db->f('manufacturer_bonus') > 0)
    $bonus = $db->f('manufacturer_bonus').' (m)';
elseif($db->f('category_bonus') > 0)
    $bonus = $db->f('category_bonus'). ' (c)';

but this, don't work:

$bonus = $db->f('item_bonus') > 0 ? $db->f('item_bonus').' (i)' : $db->f('manufacturer_bonus') > 0 ? $db->f('manufacturer_bonus').' (m)' : $db->f('category_bonus') > 0 ? $db->f('category_bonus'). ' (c)' : '0';

What I'm doing wrong? $db->f return number, float type.

PHP's `?:` operator is [counterintuitive](http://us.php.net/ternary#example-121) when you chain it. Also, the first version was far more readable. — DCoder, Dec 29 '12 at 12:38
When you don't understand it now, how can you assume you or any other will understand in later (in one week)? Just don't use `?:` this way, else you are shooting in your own foot. — KingCrunch, Dec 29 '12 at 12:41
@Luntegg Because what I said: You will see this code later and you will probably not understand it again, until you debug it. You are producing code, that is hard to maintain. — KingCrunch, Dec 29 '12 at 12:45
@KingCrunch I realized .. Well, thanks, I will keep in mind. Just wanted to make a compact code. — Luntegg, Dec 29 '12 at 12:48
@Luntegg any reason for compact code? You have hosting plan from 1997 with 2MB space? Or you are distributing your code with floppy disks? — dev-null-dweller, Dec 29 '12 at 12:59

score 1 · Accepted Answer · answered Dec 29 '12 at 12:42

1

Try it with grouping:

$bonus = $db->f('item_bonus') > 0 ? $db->f('item_bonus').' (i)' : ($db->f('manufacturer_bonus') > 0 ? $db->f('manufacturer_bonus').' (m)' : ($db->f('category_bonus') > 0 ? $db->f('category_bonus'). ' (c)' : '0'));

answered Dec 29 '12 at 12:42

TheHorse

2,787
1
23
32

score 0 · Answer 2 · answered Dec 29 '12 at 12:47

0

Its better use the ternary operators with simple if/else statements so you can understand them in te future if you need to check your code again. So in this case is better let the first piece of code you had posted.

answered Dec 29 '12 at 12:47

John Busca Longe

51
1
2

Stuart Wakefield · Answer 3 · 2012-12-29T13:11:08.713

If your reasoning is to do away with a hardcoded if statement you can separate function and data...

// Important stuff
// Change the order of bonus importance using this array
$bonus_precedence = array("item", "manufacturer", "category");
// Change the symbol for each item with this associative array
$bonus_markers = array(
  "item" => "i",
  "manufacturer" => "m",
  "category" => "c"
);

// Magic beyond here
foreach($bonus_precedence as $bonus_type) {
  // Get the next bonus type
  $bonus = $db->f("{$bonus_type}_bonus");
  // Was there a bonus
  if($bonus > 0) {
    // Output the bonus in full and break the loop
    $bonus_marker = $bonus_markers[$bonus_type];
    $bonus .= " ($bonus_marker)";
    break;
  } 
}

If it doesn't matter that it is hardcoded, stick to the long if statement. You can comment it easily and it is easy to read and comment:

// Only one bonus type is allowed
// Item bonuses are the most important if there is
// an item bonus we ignore all other bonuses
if($db->f("item_bonus") > 0) {
  $bonus = "{$db->f("item_bonus")} (i)";

// Manufacturer bonuses are the next most important
} elseif($db->f("manufacturer_bonus") > 0) {
  $bonus = "{$db->f("manufacturer_bonus")} (m)";

// Category bonus is the fallback if there are no 
// other bonuses
} elseif($db->f("category_bonus") > 0) {
  $bonus = "{$db->f("category_bonus")} (c)";
}

Otherwise at least try to make it comprehensible:

// Only one bonus type is allowed
$bonus = 

  // Item bonuses are the most important if there is
  // an item bonus we ignore all other bonuses
  ($db->f("item_bonus") > 0 ? "{$db->f("item_bonus")} (i)" : 

    // Manufacturer bonuses are the next most important
    ($db->f("manufacturer_bonus") > 0 ? "{$db->f("manufacturer_bonus")} (m)" : 

      // Category bonus is the fallback if there are no 
      // other bonuses
      ($db->f("category_bonus") > 0 ? "{$db->f("category_bonus")} (c)" : "0")
    )
  );

Thanks, but I don't think, that everything has to be so complicated :) — Luntegg, Dec 29 '12 at 12:53

Comparison Operator "IF" in PHP

3 Answers3