-1

Been playing around a bit with both the ternary operator and null coalesce these past few weeks and really enjoy the way that it's been able to simplify a lot of my logic, especially in places where I would previously have to stack a series of if (!empty($variable))-kinds of lines to an already annoyingly large if/else statement. Now I can just $object->attribute = $source->attribute ?? null, assuming I don't know if there will be an attribute in source or not.

Now, the question that I'm having issues with is trying to figure out how to best use this for logging. Say I've got a function like:

public static function addToQueue($field, $id)
{
    if ($field ?? $id ?? null == null) {
        return false;
    } elseif ($field != 'name' && $field != 'id') {
        return false;
    } elseif (Queue::where($field, $id)->count() != 0) {
        return true;
    } else {
        Queue::insert([$field => $id]);
        return true;
    }
}

Fairly straightforward; you send addToQueue() two arguments, the field and the id, and then it does three checks. Are any of them null? Then return false. Is field something other than name or id? Then return false. Is this entry already in the Queue? Then return true (since we're interested in making sure that the entry is in the queue, not whether we added it right now or not). And, finally, if the pair is not in the queue, we add it and - again - return true.

Now; so far so good; right? Doesn't seem to be an issue here, even though I see how I could probably make the logic inside of the function a little neater. The problem comes in my usage of it. Essentially, what I want to do is something like this - but with ternary operators:

if (QueueHandler::addToQueue($input->field, $input->value) == true) { $app->log->info($input->field . '/' . $input->value . ' added to queue.'; }

I want it to do something if the operation it carries out evaluates as true, but do nothing if it evaluates as false.

And yes, I know, it's called Ternary because you need three operations, but since PHP allows you to isset($variable) ?: echo 'Dude. It\'s not set...'; nowadays, I figured there should be a way to do the opposite, right?

Breki Tomasson
  • 207
  • 2
  • 11
  • 4
    Don't force use of ternarys. Ternarys should be used pretty much exclusively to assign or return exactly two values, not to carry out side effects. If you're having difficulty using them in a particular scenario, that's likely a sign that their use isn't appropriate there. – Carcigenicate Jul 30 '17 at 16:21
  • Without looking into it more (cause I just woke up); at first glance, "`null == null`" could be replaced with "`true`". But as Carcig(some more letters) says, forfeiting workiness for brevity is counter productive. – Fred Gandt Jul 30 '17 at 16:32
  • FYI: https://stackoverflow.com/questions/34571330/php-ternary-operator-vs-null-coalescing-operator – Fred Gandt Jul 30 '17 at 16:33
  • $id??null is pointless. `$id` will always be set in the function scope. – apokryfos Jul 30 '17 at 16:38

4 Answers4

2

The ?? operator is right associative (Source)

It means:

$field??$id??null == null 

If $field is not set or null then that collapses to:

$id??null==null

If $id is not set or null that collapses to:

null==null 

That expression will always be true since the null is swallowed by the ?? operator. This means $field??$id??null==null will never evaluate to a falsey value.

You need to be explicit if you want to force precedence:

($field??$id??null) == null
apokryfos
  • 38,771
  • 9
  • 70
  • 114
0

The ternary is shorthand for IF ELSE.

Even the code you mentioned isset($variable) ?: echo 'Dude. It\'s not set...'; is doing that (it will return $variable if isset is true), even though it appears you have just added the else part. So there is no "opposite" there is only IF and ELSE

For example:

$a = 'foo';
echo $a ?: 'bar'; // 'foo'
Ryan Tuosto
  • 1,941
  • 15
  • 23
0

So, you current code is simple

if (QueueHandler::addToQueue($input->field, $input->value) == true) {    
    $app->log->info($input->field . '/' . $input->value . ' added to queue.'; 
}

As addToQueue returns bool, you can simplify to:

if (QueueHandler::addToQueue($input->field, $input->value)) {    
    $app->log->info($input->field . '/' . $input->value . ' added to queue.'; 
}

Now you're trying to use this:

!QueueHandler::addToQueue($input->field, $input->value) ?: $app->log->info($input->field . '/' . $input->value . ' added to queue.';

I don't think it is more readable then previous examples.

u_mulder
  • 54,101
  • 5
  • 48
  • 64
0

Wanna play with ternarys?

Do like it:

function addToQueue($field, $id)
{
    return ($field??$id??null) == null?false:($field != 'name' && $field != 'id')?false:
        (Queue::where($field, $id)->count() != 0)?true:Queue::insert([$field => $id])?true:true;
}

No one will understand, not even you. Every time you have to change it you will ask yourself why you did this way, but it will get your phpunit code coverage easier.

Now seriously, apokryfos spotted it. But you have to ask yourself what are you getting out of using ternarys.

It's really good for things like:

function getClub(){
    return $this->isClubVisible()? $this->club: null;
}

And every operation that you would solve with an if/else but as you can see if you overuse it, it will get really messy and unreadable.