Static Code Analysis can Highlight Deeper Flaws

Static code analysis (SCA) tools like those offered by FindBugs, PMD, CheckStyle, IntelliJ IDEA can help a development team track down problems and keep quality high. But when an SCA tool flags a problem, how should a team react? Vikas Hazrati's Static Code Analysis is just the Tip of the Iceberg suggested: look deeper.

If the team agrees with the flagged problem, then they may fix the problem. In many cases, however, the flagged problem can highlight deeper flaws that are less visible and less easily detected by code analysis tools:

I got first hand insight into this when I was doing an audit for a large system which is being used online by millions of people. While doing the analysis my co-auditor suggested that [we] look for hot spots which are highlighted by the SCA tools. Then, [we could] dive into the code at those places to find out bigger issues.

Vikas cites five examples where a problem flagged by static analysis led to the discovery of deeper problems in the code. For instance, when they discovered that there were servlets with 3500 lines and 800-line methods:

A quick fix to this might have been to reduce the method size by splitting the methods and moving some methods out of the class to a helper class so that the class size and method size violations are taken care of.

However, when we looked deeper to answer the question "Why do the servlets have so much lines and such big methods?" we realized that all of the business logic was present in the servlets. There was a bigger violation of Single Responsibility Principle where all the logic was present in the single class. The presentation logic, business logic and data access logic were all clubbed together thus leading to fragile design which is hard to change without breaking. There was no layering and no separation of concerns.

Likewise, when they found methods with a large number of parameters:

A quick way to solve this and make the SCA tool happy would be to introduce an object and populate the object with the required parameters.

However, when we took a deep dive we realized that the system did not have any abstraction. There were no domain objects in the system either. When data had to be passed between method calls, all of that would be passed as primitive data, mostly strings. There was no focus on domain driven design. The methods were overloaded with every extra parameter that would be required in a different situation which suggested that the system was open for modification and closed for extension. For any small change a new method had to be written. This violated the Open Close[d] Principle.