Why reverse condition check is not practical

Small note about “reversed” condition checks like these:

if (CONSTANT == $variable) {
  // do something
}

and why their benefit is so small compared to the side-effects.

It’s stated that these checks are better because they prevent you from doing an obvious mistake:

if ($variable = CONSTANT) {
  // oops, assigment
}

Here comes the side-effects though. Suppose you have a code like this:

if ('admin' == $username) {
  // allow some admin actions
}

It looks okay, you prevented the “accidental assignment” error until one day you decide to refactor this into:

if ('admin' == $user->getUsername()) {
  // allow some admin actions
}

Now should you leave it reversed or not? The danger of assignment is gone since $user->getUsername() is not an lvalue. Suppose you leave it like this just to make all assignments similar. But then you do another refactoring and end up with:

if (User::getAdminUsername() == $user->getUsername()) {
  // allow some admin actions
}

NOW what? At what point it becomes so silly that it won’t bring any value to your code? It’s always more readable when you write if ($a == 'b') since that is how you mind works, it first evaluates something you don’t know and then tries to match it with something you do know. And if you are so obsessed with assignment error, just write a simple SVN/Git hook that blocks the commit if that happens.

Sometimes you see people using if ($variable = CONSTANT) intentionally to save the space combining two actions into one. While that is possible it’s also possible someone else from your team finds this code while debugging, considers it a source of a bug and “fixes” it. Besides, “someone else” might be you in a year. So if you write that kind of code, please add a comment right next to it.

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.