Static Code Analysis can Highlight Deeper Flaws

Posted by Geoffrey Wiseman on Dec 12, 2007 08:19 PM

Community

Agile

Topics

Artifacts & Tools,

Design

Tags

Static Analysis

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.

1. Back to top

   Some comments

   Dec 13, 2007 7:13 AM by Frédéric TU

   Well, you're completely right. SCA cannot repair design and can just found some form problem. But at least, you can measure quickly the quality of code because when the form isn't here, they may be something fishy behind! I also have been in a team where some people ask us to declare Checkstyle and to see what's hap. Finally 90% of problem are linked to where { ( and other symbol are placed, which I don't care completely!! SCA CANNOT be used without a clear procedure to fix it, and you denote well that SCA cannot be left to simple developper which can take some time to hide stupidity. One more detail, SCA must be considered as a global firm policies and must be linked with some rules of coding. Using tool to solve all by itself is completely illusion.

   Reply

2. Back to top

   Re: Some comments

   Dec 14, 2007 10:31 AM by Juan Bernabo

   SCA CANNOT be used without a clear procedure to fix it, and you denote well that SCA cannot be left to simple developper which can take some time to hide stupidity. One more detail, SCA must be considered as a global firm policies and must be linked with some rules of coding. Using tool to solve all by itself is completely illusion.

   Uhmmm... or maybe used as a tool for a concius team to help them look for trouble areas in the software and missing skills in the team..... Cheers, Juan.

   Reply

• Older >