Php-cs-fixer: Question: Why Yoda-condition fixer does senseless replacements?

[ossinkine]

Yoda condition is needed to avoid mistake when there is variable on the left side of equality expression.
So, why do we need the replacements in inequality and in equalities without variables where impossible make mistake?
For example:

count($array) === 0
0 === count($array)

count($array) > 0
0 < count($array)

I apologize in advance , if this subject already rose, give me the link please.

[kubawerlos]

Senseless or not - that's your opinion (personally I don't use Yoda conditions at all and the fixer helps me to do that). According to wiki:

Yoda conditions is a programming style where the two parts of an expression are reversed from the typical order in a conditional statement.

Thus yoda_style is to change all possible expressions to/from Yoda style.

The configuration of the fixer you can find in README allows you to specify types of statements to change, so you can easily achieve that only first example is in Yoda style.

[ossinkine]

@kubawerlos According to your link:

In programming languages that use a single equals sign (=) for assignment and not for comparison, a possible mistake is to assign a value unintentionally instead of writing a conditional statement.

I use Yoda condition and want the fixer fix it up:
$object === null to null === $object, $min === 10 to 10 === $min and so on.
But nothing more.

[SpacePossum]

If you want to have more options on the Yoda fixer it would be a FR. For more background info about how the Yoda fixer came to be including the options and the case is does (not) fix you could read up on the PR https://github.com/FriendsOfPHP/PHP-CS-Fixer/pull/2446 , the PR's it replaced and the issues it closed. However this is not likely to provide more info than @kubawerlos stated.
Closing as @kubawerlos gave some good insights on this topic.

[Synchro]

Just for the record, I felt this could use some expansion. Yoda conditions are not just a convention - They exist because they turn potential runtime errors into compile time errors, but significantly, this benefit is only realised in some specific circumstances, for example:

if ($a == 5) {

is more safely expressed as:

if (5 == $a) {

because if you accidentally typed if (5 = $a) {, you'd get an immediate compile error, whereas if ($a = 5) { would not. So that's all fine and understandable.

What is not understandable, and makes no particular sense is where this approach is applied for convention's sake, where there is no practical benefit, for example converting:

if (count($a) == 5) {

to

if (5 == count($a)) {

is pointless because in either case, typing a = instead of a == will result in a compile time error, so it doesn't matter which way around they are so there is no value in enforcing one over the other. Given that the conventional left-to-right format is generally considered easier to read (which is why many don't like yoda conditions), it would make sense for there to be an option in php-cs-fixer to only apply yoda conditions where it is likely to actually help, that is, where the left hand side of a comparison is assignable, otherwise by fixing the few cases where it does help, you reduce readability for the entire code base for no practical gain.

[ossinkine]

@Synchro Exactly!

[Synchro]

I just found that the original PR for enforcing yoda conditions (#693) contained exactly the same point, and what's more pretty much everyone involved in it seemed to agree - so I'm not sure how it ended up here, doing exactly the opposite...

[julienfalque]

Applying Yoda style everytime including when it's not necessary according to the rule you explained makes sense too: people can get used to read Yoda style conditions and prefer avoid mixing both styles.

Though I think implementing a new option like e.g. only_when_left_operand_is_assignable makes sense. I invite you to open a new feature request or, if you have enough time, implement it directly and submit a pull request.

[Slamdunk]

@Synchro I agree with @julienfalque : 0 === count($var) appears to be useless but instead makes sense.
Imagine the first revision of the code to be:

if (count($var) === 0) {
    // do stuff
}

One may think yoda condition only raises read complexity without any gains.
But then you need to refactor the code like:

$count = count($var);
if ($count === 0) {
    // do stuff
}

And the fixer must fix it:

$count = count($var);
if (0 === $count) {
    // do stuff
}

Another refactor (it happens), and you revert the logic:

if (0 === count($var)) {
    // do stuff
}

Maybe you have more condition like this around:

if (0 === count($var)) {
    if (strlen($text) === 1) {
        // do stuff
    }
}

And you end up in a mixed mess.

The solutions are:

1. The developer remembers to switch back to non-yoda all useless yoda conditions
2. The fixer forces useless yoda condition to be non-yoda
3. You either choose yoda or non-yoda everywhere, without exceptions

To me the most clear to read, understand and follow is the last one.

[Synchro]

Now apply the same logic to other comparison operators (which the fixer handles), and it invites disaster:

if (count($var) >= 5) {
    // do stuff
}

Needs to become (assuming we're doing exception-free Yoda):

if (5 < count($var)) {
    // do stuff
}

That kind of thing is extremely likely to introduce unexpected errors, represents a major drop in readability, and it doesn't do anything to convert a runtime error to a compile time one. In short, useless in every practical way other than to conform to some rigid dogmatic rule.

Also, if you're using === rather than ==, you're much less likely to accidentally mistype it as = anyway - which is after all the entire point of the exercise.

[SpacePossum]

Now apply the same logic to other comparison operators (which the fixer handles), and it invites disaster:

why would you apply it if you don't like, it is even off by default?

[Synchro]

Uh, @slamdunk said "yoda or non-yoda everywhere, without exceptions" was the clearest and most understandable choice; I was just showing that that's not true.

[SpacePossum]

rigid dogmatic rule

While l do like hearing suggestions from people about how we could improve something or what we might have done wrong, I've to say I'm done with comments like this.

What we provide here is both free and optional, no one forces you to use it.

Don't like it? Here is how things can be improved:

• provide clear and simple examples, so no lengthy discussion going back and forth between multiple closed issues and PR's
• open a PR and see it through till its merged