Do not blindly use the ternary operator; it always returns copies of non-object values

Yeah, but we avoid monoline if / if-else constructs everywhere else in core

suppose this task is a release blocker

Wow, very interesting and incredibly dumb from PHP! ack '\?.*:' |wc -l returns 3481. That's going to be a bear to find the array ones and the frequently called ones :/

Updated issue summary.

I played with some various combinations of repeated grep pipes involving '[' thinking they would be involving arrays and would need a close look. Also reverse grep -v for '->' thinking that those would be in reference to objects and would not need to be investigated. But then I was thinking that a line could be referencing an array that is part of an object, so eliminating lines with -> is not useful.

I was trying to either get a subset of ones that for sure would need to be looked at, or a subset of ones we could exclude looking at. I dont think I had much success in either task.

Another though I had was how to track what has been evaluated so that people dont keep looking at the same lines over and over.

I had a (bad) idea of putting /* ternary ok */ at the end of every line someone had looked at.

Really, the line needs to be seen in context, so for example, the type maybe given in the docs or parament listing can be seen.

I'll come back later and mark which of those greps from the history I found useful.

I've never understood this pattern at the end of a function:

  if (isset($definitions[$name])) {
    return $definitions[$name];
  }
  else {
    return FALSE;
  }

I think it's clearer to use:

  if (isset($definitions[$name])) {
    return $definitions[$name];
  }
  return FALSE;

It's at least better than with the else { } for converting end of function ?: returns.

Patch for field.module : #1866270: Cleanup ternaries in field.module

Updated issue summary.

This is a micro-optimization.

I would only remove uses of the ternary operator when it doesn't negatively impact readability. We should not just do it as a rule.

Also, in places that we do make the change, we need to verify it with xhprof before/after.

Really, we should be creating issues for each area where this is changed, so we can measure the performance difference independently.

For the same reason we should probably be passing/returning arrays by reference when it's not dangerous.

Posting it here since I think it's more relevant to this issue, but I profiled the before and after of the fields patch which removes ternaries.

I made a view that list 10 nodes of a custom content type with 9 fields, and then called that page 100 times with and without the patch.

Wall time

Memory usage

Peak memory usage

Wall time difference is less than half a millisecond in the functions where it makes a difference at all, and a there's slight difference in memory usage.

These are barely worthwhile improvements at best, readability regressions at worst. It's possible that there are a few areas in the critical path where it could be worthwhile, but we have to evaluate those individually. Encouraging dont-do-this-because-its-slow rules like this to developers is harmful. The expressiveness and readability of our code is far more important than micro-optimizations.

Tested on php 5.3.17, OSX.

I dont think my results are vastly different, I'm just showing it in the context of our application.

I've previously tested something similar to what Fabien did, where you create a large array and then iterate over it many times. In that scenario, the problem is magnified, so the results are more dramatic. For a project like Twig, that's a reasonable consideration. Like I said, we may have spots like that, but most of the time it won't make a significant enough difference to warrant a change.

just noticed this. holycrap, php--

my concern here is that we could have some potentially very nasty, incredibly-difficult-to-find bugs if, say, a value stored in drupal_static() was to be used in a ternary, which then never gets propagated back to drupal_static() because the reference link was silently broken. IMO, those are the cases we should really check for...because otherwise, i'm in agreement with #10.

Removed unnecessary else control structure construct.

Only in rare and deliberate, well thought out cases do ternary operators improve readability in comparison to a simple if else block.

Take for example this code in theme.inc

return '<a href="' . check_plain(url($variables['path'], $variables['options'])) . '"' . drupal_attributes($variables['options']['attributes']) . '>' . ($variables['options']['html'] ? $variables['text'] : check_plain($variables['text'])) . '</a>';

Or this in date_api.module:

    return $display_ago ? t('!time ago', array('!time' => format_interval($interval, $granularity))) :
      t('!time', array('!time' => format_interval($interval, $granularity)));

I have seen far worse in core too, can't find it right now. With multiple levels of nested ternaries combined with a distinct lack of comments.

In order to evaluate what the code did I had to manually rewrite it as a set of nested if else statements.

Just hit this in an unrelated context and found this issue. Thought it was worth noting this is now fixed in PHP 7.

Hi Sun,
In earlier version of php, It was big performance issue. But now you can use ternary operator blindly.

Ternary operator performance issue has been resolved after PHP 5.4. You can check the release notes at the below url.

http://php.net/releases/NEWS_5_4_0_beta2.txt

Ha! Go PHP. :-)

Since Drupal 8 requires 5.5 at least, I'm going to mark this won't-fix as avoiding ternaries is no longer useful.

Gonna push this one to 7.x for now. Just fixed in PHP-7 still means 5.2-5.6, etc. are problematic.

I sometimes had strange memory effects, so I want to check if that was caused by this, which I had forgotten.

FabianX: No, it was fixed in PHP 5.4 according to #21. PHP 5.5 goes out of support today. If you're still using PHP < 5.4, strange memory effects are the least of your problems at this point. :-)

I'm closing the issue based on:

-The problem was fixed in PHP 5.4
-PHP 5 now unsupported
-3 years from last comment