#5596 - Fix unnecessary "else if" nesting
| Identifier | #5596 |
|---|---|
| Issue type | Feature request or suggestion |
| Title | Fix unnecessary "else if" nesting |
| Status | Closed (rejected) |
| Tags |
Roadmap: Over the horizon (custom) Roadmap: v11 partial implementation (custom) Type: Standards compliance (custom) |
| Handling member | Deleted |
| Addon | General / Uncategorised |
| Description | PSR-12 does not allow "else if", but this also includes uses like this:
if ($something) { } else { if ($something_else) { } } This can (and MUST according to PSR-12) be merged into... if ($something) { } elseif ($something_else) { } Fix all of these occurrences, document it in coding standards, and try to get code quality checker to pick up on these if possible. |
| Steps to reproduce | |
| Funded? | No |
The system will post a comment when this issue is modified (e.g., status changes). To be notified of this, click "Enable comment notifications".
Comments
I'd advise against any kind of slavish interpretation of standards- if an if-else was structured like this then most likely it was for a reason.
Background...
For a set of different outcomes based on some value you'd use switch...
switch ($foo) {
case 'a':
something();
break;
case 'b':
something_else();
break;
}
But often different branching is more complex than just one variable, so we have elseif...
if ($a == 'whatever') {
something();
} elseif ($b == 'foo') {
something_else();
} else {
blah();
}
But there are situations where things are computational equivalent to the above, but not semantically, e.g...
if ($a == 'whatever') {
something();
} else {
if ($b == 'foo') {
something_else();
} else {
blah();
}
}
It all comes down to intent. Are we providing a series of prioritized alternate branches, or does it just happen that one of those branches itself has some nested logic that COULD be folded as another branch but is logically separate enough to be coded as a subbranch.
For code to be readable, I think the semantics of the code are just as important as coding standards or syntax.
I believe the if / elseif standard is better for the following reasons:
1. Readability can be clarified with an inline comment and ensuring variable names are clear and concise
2. It eliminates [at least] one additional point of computation (e.g. having to process both the else and the nested if instead of only the elseif)
3. The code looks cleaner because we don't have an unnecessary code indentation for the nested if.
4. The code complexity is reduced as well; nested ifs are considered complex and, for good reason (additional computation, reduced readability, added logical complexity), to be avoided whenever possible.
5. It's counter-intuitive to use nested ifs especially when we try avoiding using too many nested ifs to begin with by using ifs which return instead.
2 - You'd never be able to measure that, it's not even a microoptimization, more of a nanooptimization ;). And, in theory PHP could optimize it away if it hasn't already.
3 - That's subjective.
4 - Well, the logical code complexity is pretty much identical, if all we are doing is shuffling in and out of one level of indentation. It's true multiple-levels of ifs generally correlates with additional complexity, but not in the cases where it would be practicable to remove said indentation. Readability and Computation already covered in prior points.
5 - Counter-intuitive seems a bit subjective too; to me it's more intuitive to separate out logic into trees, where each level of the tree is dealing with one decision domain. Actually you embedded a coding paradigm which itself is controversial in your justification here - the idea of a function having multiple returns. See https://en.m.wikipedia.org/wiki/Return_statement#Multiple_return_statements
All this said, I don't care that much, but I don't believe restructuring code is a very productive use of time given other considerations.
Generally, most developers I've researched agree that if / elseif is more readable than nested ifs.
However, I've found a big exception to the rule: if an if / elseif contains multiple elseifs, the readability and computational benefits go out the window, and arguably nested ifs are actually more efficient in that case.
At the end of the day though, this is indeed pedantic. Furthermore, there are a few conditions in which an if / elseif should not be used (which is probably enough to consider it not worthy of using as a standard):
1) What I mentioned above with having multiple elseifs
2) If the conditions themselves are complex or significantly semantically different from each other
3) If any code, even so much as setting the value on a variable, is shared between more than one if condition (avoiding repetition is generally more important than avoiding nested if statements)
Thus, I'm going to close the issue. I may use if / elseif still when applicable and none of the above conditions apply. But there are too many exceptions, and is too pedantic, to consider it a worthwhile standard.