For several years now I am a big fan of using static code analysis tools for checking the source code quality. We are mostly doing C# development so NDepend was the best way to go for me. Most of the time I manually perform the analysis, for example once every two weeks.
Lately I have had a discussion with a colleague whether we should include NDepend in our continuous integration process so that the build automatically turns red if for example a method has a too high complexity.
My opinion hereby is that it's not possible to define metrics that are valid for the whole code. I think that you would either get way to many "false complaints" – code that generally is ok but does not match the metrics. The following code for example has a very high complexity, even if I think that sometimes you can't prevent it.
if (remoteResult == RemoteResult.Dummy1 || remoteResult == RemoteResult.Dummy2 ||
remoteResult == RemoteResult.Dummy3 || remoteResult == RemoteResult.Dummy4 || ...
Of course it would also work to increase the thresholds of the metrics, but in this case I would loose valuable information.
Another example for example is the maximum number of members for a class. I would like to keep this lower than 10 for classes with lots of logic. But on the other hand there are many "domain models" with more than 10 properties. I wouldn't like to extract another class just because of the metric – as this would lower the readability and does not improve it which should be the goal of code quality metrics.
Do you think automatically checking code quality via metrics is possible? If yes, for all or only for specific metrics? (A list of common NDepend metrics can be found at http://www.ndepend.com/Metrics.aspx )
Best Answer
It is definitely impossible to automatically check code quality, you can only check for some minimum code quality.
Even "experienced" programmers are having trouble to formalize what actually constitutes code quality. And even among the "most skilled" ones, there's a high divergence in opinion on what is good or bad.
There are some obvious rules that shouldn't be broken. A thousand lines in a method is definitely too much. A thousand members in a class is also. Nobody would argue on that. Most people would choose for smaller numbers. I personally rarely have a file with more than 200 LOCS, but it largely depends on personal taste, habit and also the programming language used.
The thing is, automatic code analysis tools can only use rather simple criteria. Therefore, not unlike testing, they can never be used to ascertain code quality, only determine horrible lack of it. If there is an absolute way to define code quality, I would think it means a sensible ratio of complexities between a solution and the problem that it solves.
However you do not care for code quality as such and do not seek its absolute value - well, you personally may (I hope you do), but the people who pay you generally don't care. What you really care for is maintainability and ideally a low bus factor.
And this is something relatively subjective, depending a lot on the people in your team. They need to be able to look at a piece of code and understand what it is doing. And that can ultimately only be achieve through code reviews.
So while you can implement automated means to uncover obviously bad code, you cannot implement such means to really assure good code. Only code reviews can do that (or at least drastically help getting towards that goal) and they also definitely protect you from code that is obviously bad.
So it really depends on your team (size, continuity, etc.), whether or not such an automated check makes sense. It might as well save time to do more important things in reviews. Or it might just piss people off to no end. In the long term you should find the discipline to not commit super-obviously-bad code in the first place. It's really not that hard and it really pays off.