The Tools of the Trade
A lot of my frustration with developers is that they don’t seem to want to improve themselves. A lot of bad excuses come from all parts of an organization, developers, testers, SCuMs and management all seem to be harboring under the delusion that even defective ways of working is better than introducing change. This is to be expected in some sense, we all like routines, even the bad ones, since routines give us a false sense of stability and control. In this post I hope to introduce a new routine for you, one that will actually help you introduce less bugs in your code and make your code better.
Static code analysis #
This is one of the three basic testing tools that should be used by developers, the other two are unit tests and code review. Together they form the the basic foundation to ensure that your code behaves and works as intended.
Static code analysis come in many forms, as internal tools within your IDE, included as targets in build tools or as external tools that verifies code being checked in or built. The function of these tools is to verify how well your code conform to accepted best practices and how well it avoids introducing well-known bugs. This is done by checking rules and analyzing your code for blocks that break these rules.
The problem with static code analysis #
I think most of us are aware that rules are far from perfect, every rule has exceptions and as such, static code analysis is far from perfect, but that’s why there are two more tools available to us (unit tests, code review).
The use of static code analysis is the first step to make sure your code is not doing anything crazy, like introducing unused variables, missing null checks and similar. This combined with proper unit testing will give you a solid static verification of your code. Code review is then to be used to do what’s known as a dynamic analysis of the code and then to be handed over to testers for some more complex tests to be run.
The problem as I said is that static code analysis is done with the aid of rules, these rules do not always make sense and in some cases they are not applicable to the context. But this is where your skills as a developer and the aid of dynamic analysis come into play. Use a static code analysis tool to find possible errors, then use your head to make sure they are valid concerns before you correct them.
An example #
I will use IntelliJ and a plugin called SonarLint in this example to demonstrate what static code analysis can do for you and why you should use it.
I have selected a file at random and looking at the SonarLint plugin output, I see the following errors.
- Replace this if-then-else statement by a single return statement.
- Complete the task associated with this TODO statement.
- Remove this unused “log” private field.
- Merge this if statement with the enclosing one.
- This block of commented-out lines of code should be removed.
The first complaint turned out to be this one
if (!value.equals(oldState.get().getCurrentValue())) {
return true;
}
return false;
Obviously, this is a big no-no, it it hard to read and is over complicated, so let’s correct this to
return !value.equals(oldState.get().getCurrentValue());
The next error we get is an unused variable, so we just remove the private log variable. We also see an error about commented out code, so we remove that as well, because code should not be commented out, we have a version control system for that.
We also see an error about an if statement that can be merged with the enclosing one, the offending code is this snippet
if (oldState.isPresent()) {
if (!value.equals(oldState.get().getCurrentValue())) {
return true;
}
}
return false;
This we obviously refactor into
if (oldState.isPresent() && !value.equals(oldState.get().getCurrentValue()) {
return true;
}
return false;
I’m going to leave the TODO as is, because I will fix it later, but already we have tweaked the code for greater clarity and readability. Now, what exactly have we done and how will these changes improve the code?
Boolean logic #
The first example is a complication of boolean logic, we have something that already evaluates to a boolean and then we put it into an if statement to return a boolean. This is called redundancy and it should be avoided.
Unused code #
Code that is not being used just adds complexity, size and confusion. With the introduction of versioning systems the need to keep old code disappeared. Also, the old code may not be the best way to solve the issue, remove it to declutter your code and if it turns out to be needed later, just cherry-pick it from the older commit. Even better, write a better solution.
Merging if statements #
This may not be as obvious as the rest of them, but why do an and
statement in two steps? It removes clutter and it may even clarify the code to make understanding of it easier. Multiple if statements can be tricky to read and sometimes they become clearer when aggregated into a single statement.
Conclusion #
The use of static code analysis is great, it helps you reduce complexity in your code, but it is by no means a perfect tool, use your brain and have a look at what the tool gives you. If it makes sense, use it, but don’t ever think it’s a replacement for you.