RSS
 
15. Oct. 2015

Avoid Yoda conditions

15 Oct

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 (97% score) - 5 votes
 
5 Comments

Posted by Mark in PHP

 

Tags: , ,

Leave a Reply

Tip:
If you need to post a piece of code use {code type=php}...{/code}.
Allowed types are "php", "mysql", "html", "js", "css".

Please do not escape your post (leave all ", <, > and & as they are!). If you have encoded characters and need to reverse ("decode") it, you can do that here!
 

 
  1. Sven

    November 22, 2015 at 05:21

    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. Nemesis

    October 13, 2016 at 12:43

    But it is faster to read when if (‘blue’ === $color)

     
  3. Mark

    December 1, 2016 at 02:09

    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.

     
  4. Cortex-

    June 12, 2017 at 20:24

    if (true !== is_numeric($var)) {

    should be wrote

    if (!is_numeric($var)) {

    You use php-cs-fixer? Funny, Yoda condition are at true by default (see http://cs.sensiolabs.org/) and are recommended in http://symfony.com/doc/current/contributing/code/standards.html

    You should talk about switching

    @Nemesis switch ($color) { case ‘blue’: can also be fast to read.

     
  5. Mark

    July 11, 2017 at 10:44

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