Empty if() statements in your code – keep them or dump them?

When using the static Java code analysis tool PMD, one of the basic rules that you can violate is the empty if statement rule, which has a critical severity level by default. Why is there such a rule in the first place and is the critical severity level warranted? As an avid user, or abuser in terms of PMD, of empty if statements, with no clear answer, I went and trawled the web for answers.

To start off with, PMD does offer the following advise, in the Best Practices page

Generally, pick the ones you like, and ignore or suppress the warnings you don’t like. It’s just a tool.

In the context of the subject matter of this post, that may be a hint to buy the official book, PMD Applied, which is fair enough. In any case, lets get onto the first obvious example of why the rule is warranted and motivate why it should be a critical violation:

if (firstCode != null && firstCode.getCode() != null && firstCode.getCode().equals("101")) {
    doSomething();
} else {
    // TODO: process code "102"
}

Another version of the above is of course the version without the TODO comment:

if (firstCode != null && firstCode.getCode() != null && firstCode.getCode().equals("101")) {
    doSomething();
} else {

}

Both cases are clear problem areas, as the code is blatantly incomplete, and if your unit tests don’t pick it up, you certainly want your static code analyzer doing so. But what about the following:

if (firstCode != null && firstCode.getCode() != null && firstCode.getCode().equals("101")) {
    doSomething();
} else {
    // only supporting 101 in this release, 102 onwards to follow
}

In this example, some value is being added in terms of comments, and I have regularly been taking this approach, until PMD forced me to think about it. I took the approach in part because from what I can remember from compiler implementation theory any given compiler, such as a Java byte code compiler, would be clever enough to detect and remove the dead code.

As it turns out, from some limited research, this is not necessarily the case, since if it was, then shrinking tools such as ProGuard would surely not list “Remove unnecessary branches” as a byte code optimisation feature.

I managed to find very few opinions on this online, although the following was of interest. In the end, I concluded that the gain provided in the documentation in the final example does not add sufficient value to justify the risk of not completing a branch, and also, it bloats the code base. So, I have resolved to do away with the practise and to move such comments either above the if statement or into the javadoc where appropriate.

Advertisements