Consider the following block of code from the program I’m paid to maintain (and which is slowly causing me to lose all my hair).
public function saveFromForm(array $values = array(), $id = null, $issue = true) {
$a = $this->formFieldsToModel($values, $id);
$a->has_changes = true;
$issue and $a->issue(false);
}
I snipped some unimportant lines for purposes of this rant. Note the last line in the example, “$issue and $a->issue(false);”
I call this sort of coding many bad names in my less than charitable moods, but what it boils down to is it smacks of conceit, arrogance, and shows that the author was clearly in love with his own intelligence. This is a problem I see from time to time and I used to catch myself doing it in the past, though it’s less often now.
Code like this technically works - if you can’t see how it works look up lazy evaluation. And there is a time and place for lazy evaluation. This isn’t it. The line is no better than the intuitive
if ($issue) {
$a->issue(false);
}
All it does is increase the difficulty in parsing the code ever so slightly for no real reason except to ‘show off’ to whoever reads the code. Considering the author of this code consistently makes tyro level mistakes throughout his code and its design I’m not impressed - if anything the showing off like this lowers my opinion of him.
Programming languages have a lot of room for this nonsense. The worst example is nested ternaries.
$a = $b ? $b : $c ? $c : $d ? $d : null;
If $b is 0, $c is 1 and $d is 4 what is $a? Most of us need to stop a moment and think about that. Such code is painfully difficult to debug and mistakes are far too easy
$a = $b ? $c ? $d ? $b : $c : $d : null;
Again, if $b is 0, $c is 1 and $d is 4 what is $a? Everyone really needs to stop a moment and parse the second one out. But given how similar the two are it is easy to mistake one for the other. The chances of one morphing to the other through typo are quite high.
Assignment in conditional is another case of this, though more into the gray area.
if ($a = foo()) {
// code when foo returns true or anything that evals to true.
} else {
// code when foo returns false.
}
Personally I will do this when catching function returns, more likely as an escape when the function returns false.
if ( !$a = foo() ) {
$a = $default;
}
But on the whole, it’s something to avoid because it isn’t clear. There’s something to be said for coding clarity.
Cause clear code will impress the writer’s intelligence to me far more than the tomfoolery of unnecessary lazy evaluations, nested ternaries and other jerk moves which only serve to confuse whoever has to read it.