PHP

Avoid Yoda conditions

After blogging about the semantic importance of void as pseudo type I feel like I should also point out the nonsense of Yoda conditions.
Those are still used quite a bit in the coding world, mainly in and around WordPress and Symfony if I recall correctly.

First of all, what is it?
It is the switching of order in conditions:

if (2 === $value) {
}

Like u say “If blue is the sky”.

Sounds weird? Sure is 🙂

Why do they exist?

It seems they have been invented to prevent "some" mistakes (some as in only a small subset!) when writing conditions.
If you, for some reason (…?), forget to write the second = in == it will result in a parse error for a few cases, telling you that quite early in the development process.

What is the danger of using them?

The main question about coding standards is always the same. Do we write clean and readable code for the computer or for humans.
The computer usually doesn’t care about any of those standard issues. The code functionally behaves the same.
But if humans have a hard time understanding/reading the code, that will be a huge problem for maintainability.
It also introduces a high risk of making errors.
So actually, in my many years of programming I have actually seen more mistakes made because of Yoda then without.

Think about this

if (true !== is_numeric($var)) {
}
// vs.
if (is_numeric($var) !== true)  {
}

or even

if (2 < $value) {
}
// vs.
if ($value > 2) {
}

You can easily forget to negate a value when in the not so natural order or confuse the comparison direction.

Often times I see them used only for the equality checks, not for the rest of the comparisons, and that makes it super inconsistent.

if (false === $foo) {
    ...
} elseif ($bar > 2)  {
    ...
}

Eliminating the root problem

It seems the Yoda fans have simply forgotten to look for the root issue they try to prevent here.
And instead of fixing that, they made a half-hearted workaround that misses out on most accidental assignments and introduces
the right of human error due to the mental overhead involved.

Actually, it gives you a false sense of security.
What is won if you still can by accident do

// Autsch
if ($variable = $otherVariable) {
    // ...
}

So let’s fix it – the right way.

Preventing inline assignment

Prevent what you want to "hack avoid" consistently and without exceptions:

// Conditional assignment is disallowed
if (($variable = $this->foo()) === null) {
    // ...
}

// This is how it's done
$variable = $this->foo();
if ($variable === null) {
    // ...

Add a sniffer to be sure

Now add a sniffer to your CS (php-cs-fixer or phpcs tool) and automatically prevent this from happening ever
(100% error free, cannot be missed by human error).
Make sure, you cannot merge until the sniffer shows a "green light".
That means = to check for only, as all other operations are not assignment operators.

And now we can add a second sniffer to check on no primitive value is the first argument: 'foo'|true|false|null|, numbers (1…x), constants and CO.

Setup your IDE

IDEs like PHPStorm can detect and warn at runtime about inline assignment:

Settings => Editor => Inspections => Probable bugs => Assignment in condition (check)

Benefit

The chance of assigning by accident went from likely to not possible.
In the meantime readability increased a lot, as the natural language makes this "natural" in the way we read and understand the conditions without having
to do further calculations in our head.

Further reading

blog.codinghorror.com/new-programming-jargon says it all.
This is also valued by some of the most important PHP open source frameworks, like CakePHP.

It is also part of PSR-2-R Additions.

Update 2015-12

I stumbled upon an article where they also relized the same thing.

5.00 avg. rating (98% score) - 6 votes

5 Comments

  1. Hi Mark,

    thanks for your article, it really nails down the problem really well.

    In my experience the source of the problem might be programmers who started with languages like Java and learned to ‘always put constants first’ when doing something like constantObject.equals(test)

    It might be one of those patterns that got stuck into your head and which one is still applying in scenarios where it does not make any sense because you forgot about the ‘why’ in the first place.

  2. No it is not faster. It is reverse which is actually only as fast the "right way" once you are used to think backwards. But never faster.

  3. Cortex-
    What Symfony does often times does not meet modern standards. Especially regarding their coding styles 🙂
    So no, sry you might want to switch.

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.