PHP If Shorthand Best Practce

Question

Just a weird PHP question about best practice. Assuming the following function:

function get_option($val) {
  return false;
}

I want to assign to a $locale variable, the value returned from this function and, if false, set to a default en_GB one. I discovered 2 option for achieving this goal:

1st Option:
$locale = ( $locale = get_option( 'language_code' ) ) ? $locale : 'en_GB';

2nd Option:
$locale = get_option( 'language_code' ) ? get_option( 'language_code' ) : 'en_GB';

I would like to know which one is more correct and why.

Thanks

First eliminates a function call, but otherwise there is no definitive answer, so not really a good question for SO — Mark Baker, Dec 07 '15 at 13:01
Assuming that ``function get_option($val)`` does something more complex of course — andreasonny83, Dec 07 '15 at 13:03
I dont like duplicate same func call, and I dont like the first one as its not that straight forward :/, yeah, so I dont like both :( — Andrew, Dec 07 '15 at 13:07
maybe its time to fallback to `if/else` block, imho....its a opinion based question :) — Andrew, Dec 07 '15 at 13:13
with `if/else` block he still need to get the value twice. The better way would be storing the value to a variable(which is `null` by default), and then if the value is `null` then we can execute the function. i have updated my answer with this solution. — AvikB, Dec 07 '15 at 13:20
PHP7 has the Null Coalescence Operator, so `$locale = get_option( 'language_code' ) ?? 'en_gb';` if `get_option( 'language_code' )` returns a null rather than a boolean — Mark Baker, Dec 07 '15 at 13:37
IMO, `get_option()` should return the default value. That would eliminate the need for the messy ternary. — DFriend, Dec 07 '15 at 13:42
@DFriend, I totally agree with you but ``get_option()`` is a WordPress function so, unfortunately, I have no control over that. — andreasonny83, Dec 07 '15 at 13:52

aross · Answer 1 · 2015-12-07T14:12:06.313

The second option is better, but even better would be to use a shorthand ternary

$locale = get_option('language_code') ?: 'en_GB';

If your function returns only locale strings or false, this is the correct solution (and it doesn't require PHP7).

However, as mentioned in the comments, it might be an idea to return default values directly from the get_option function for a more architecturally sound solution. That means the caller is not made responsible for setting the default. Just read that you're using Wordpress and have no control over the inner workings of the function, but the advice in general still stands

Karoly Horvath · Accepted Answer · 2015-12-07T14:09:37.403

2

Both seem a bit verbose to me, to avoid duplicated calculations, I would prefer the first one (perhaps splitted to 2 lines of code).

You can create a helper function, this one has false hardcoded, but you could even pass it as a parameter:

function use_default_false($var, $default) {
    return ($var !== false) ? $var : $default;
}

Then your code becomes:

$locale = use_default_false(get_option('language_code'), 'GB');

Since PHP5.3 you can use the shorthand ternary operator ?:.

Be aware that it will check the left-hand side argument for truthy, which prevents it to be used if the valid value you're checking evaluates to false (e.g.: 0, "", "0", array()...). Because of that, I wouldn't generally recommend it, but in this case I assume the locale is a non-empty non-"0" string, so it should be fine.

$locale = get_option('language_code') ?: 'GB';

With PHP7 you can use the null coalesce operator ??.

It checks for NULL so you have to alter the default value returned by your function.

$locale = get_option('language_code') ?? 'GB';

edited Dec 07 '15 at 14:09

answered Dec 07 '15 at 13:41

Karoly Horvath

88,860
11
107
169

1

Why not simply a shorthand ternary? – aross Dec 07 '15 at 13:44
@aross: because that check if something *evaluates* to false. You couldn't use it with perfectly valid things like `0`, `""`... If you know those aren't valid values returned by the function, then it's fine. – Karoly Horvath Dec 07 '15 at 13:51
@KarolyHorvath in this case, I think the language code **must** contain something. This is a non-sense to accept `0` or `""` as a language code. Maybe you should propose the operator `?:` that is supported since PHP 5.3 – Laurent W. Dec 07 '15 at 13:57
@L.Wartel: It was intended to be a more generic answer, but I will update it, sure... – Karoly Horvath Dec 07 '15 at 14:02

AvikB · Answer 3 · 2015-12-07T13:12:21.627

-1

I would prefer the second one

basically if get_option( 'language_code' ) returns true then get_option( 'language_code' ) execute else other option.

it is easier to understand, and maintainable.

for duplicate code issue use something similer to this:

you need to post some more code, but here is a better way to do it:

var var1 = null;
function get_option( somevar ){
if (var1  != null) {
            return true;
        } else {
              var1  = do some stuff;
              return true;
        }
}

and then call the function like this

$locale = get_option( 'language_code' ) ? var1 : 'en_GB';

edited Dec 07 '15 at 13:12

answered Dec 07 '15 at 13:05

AvikB

355
2
12

Right but you repeat the ``get_option( 'language_code' )`` twice in the same line of code which is not really good from a performance point of view – andreasonny83 Dec 07 '15 at 13:06
@AndreaSonny i think you need to use better way to implement `get_option( 'language_code' )`. i have updated my answer. this is how you can avoid code duplication. basically you are caching the value to `var1`, and then check if if it is null and run it. – AvikB Dec 07 '15 at 13:15
Ok, that would fix the repetition issue but you're writing a lot of code for achieving the same result. – andreasonny83 Dec 07 '15 at 13:20
this is **Maintainable**, and readable. also you can always get the value from `var1`, if you know it is not null. This is the best approach for this kind of situation, specially you can reuse `var1` and also you can check if it is null or not. otherwise you might need to do it later in the code. – AvikB Dec 07 '15 at 13:25
I don't agree on this. Using my code: ``$locale = get_option( 'language_code' ) ? get_option( 'language_code' ) : 'en_GB';``, ``$locale`` will never be ``null`` because I define ``en_GB`` as the default value – andreasonny83 Dec 07 '15 at 13:26
@SonnY also this is not `a lot` code. you just have to cache it in one var and true false will return from the function. Which i am SURE you already have done. This will save code duplication. – AvikB Dec 07 '15 at 13:27
this is mostly a personal preference, but code duplication is **NOT THE BEST PRACTICE**. – AvikB Dec 07 '15 at 13:29

PHP If Shorthand Best Practce

3 Answers3