From time to time I see the following code pattern (note line 4):
$isTransportable = !$item->material->isHazardous()
&& !$item->container->isPressurized();
if ($isTransportable == true) {
$transporter->transport($item);
}
Sometimes, this may be due to changes in legacy code applied without looking at the bigger picture. However, other times this seemed more intentional than accidental, in which case I consider it a bad pattern for the following reasons:
- Is true really true? Maybe wrap it in some brackets and check that the result is true?
- It is not idiomatic – in practice, you’d say “if transportable, do something” not “if transportable is true, do something”
- It is harder to read: variable, operator and value – what one might call as visual debt
- It is unnecessarily verbose – the short version is just as readable
- It is technically ambiguous – this is more of a problem with PHP, however that condition actually checks against truth-y values (true, numbers except 0, non-empty strings, non-empty arrays, objects)
So I should just switch to identical operator (===), right?
Not necessarily. In cases where you specifically expect a boolean state, there’s no reason to be pedantic about the real value. Remember, garbage in – garbage out: If you get a 1 instead of true, the behaviour might not be as desired either way. Also, if you really have to have some checks, put them at the very top and throw an InvalidArgumentException
instead (or use type declaration in PHP 7).
But I don’t like boolean type coercion
Deal with it 😎. It’s not just PHP that allows type coercion and nearly every language that does also agrees more or less on the behaviour (of course, as usual PHP has it’s own special behaviour). Anyway, you can always ensure the type upfront instead of verifying values inside conditions later.
What if I want to check for a false? An exclamation mark is too short
And? The exclamation mark as a logic negator is a very common operator in many languages. Also, if you really want to be more verbose and idiomatic, you can also use not: if (not $isTransportable) {
.
Bottom line
Thanks to some standards and accepted practices, one can write concise code without worrying about making it unreadable. Verbosity might help a novice developer, but in the long run will make large swats of code larger and harder to read by any audience.