Bad Smells When Reviewing Code Affects Approach?

Question

G'day,

I was thinking about a comment from Kristopher Johnson about my answer to this question regarding Software Development Quality.

I'd posted a list of software quality metrics that I could think of off the top of my head that included:

1. McCabe Cyclometric Complexity - basically a measure of the number of linear paths through code.
2. Levels of indentation - a measure of complexity when looking at nested decision statements.
3. Distance from declaration to first use - how many statements exist between where a variable is declared and where it is first used.
4. Comment percentage - how many lines of code are comments compared to source code.
5. Percent test coverage - as a percentage of lines of code, how many are exercised by your suite of tests.
6. Path test coverage - how many paths of execution are exercised by your tests.
7. Unit coverage - how many individual units, classes, packages, etc., are exercised by your unit tests.

Kris's comment was:

Only the test-coverage metrics listed here could be considered a measure of "quality." The others are measurements of complexity and readability, which really has nothing to do with quality.

Apart from the fact that I don't agree with this statement at all, it got me thinking.

When I have to review code that has hardly any associated tests, whether unit, system or integration, I tend to approach the code much, much more warily than if I see a good suite of tests that have been successfully passed.

Same thing when performing security audits on code. If I see unused variables, huge functions, bizarre mixtures of configs, per server, per dir, etc. being used in Apache modules it also predisposes me to approach the code very warily.

Does anyone else use this initial "gut feeling" approach and does it affect the outcome?

BTW I don't agree with Kris's comment because all the other metrics are definitely valid measures that will help highlight badly designed, poorly executed code. As Damian Conway says:

Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.

Answer 1

I think you and Kris just disagree on the definition of quality. Take the analogy of a proof in Mathematics.

You could argue that quality is just down to whether the proof is correct, that is it correctly goes from assumptions to result. However, most mathematicians would agree that some proofs are better than others because they are shorter, or more cleverly done, or easier to understand, and these are measures of quality. Only the first definition is formally definable, but most Mathematicians I think would mean the second if they said "a better proof".

What Kris says is true under the first definition, only tests really measure correctness, but I think most programmers, including me, would associate quality with your measures as well.

Answer 2

Developed "gut feeling" is what distinguish beginners from professionals. After you gain some experience "gut feeling" becomes one of the main contributors to final decision. It doesn't matter whether you're reviewing somebody's code or creating system architecture, gut feeling guides you. However pragmatic developer must not be too self-assured. There is always a place for check-lists and other means.

As for metrics, I totally agree with you. Metrics is meaningless if it doesn't contribute to code quality.

Answer 3

Yes, the "gut feeeling" is a very good tool as soon as you have a little experience. I seem to remember that Hunt and Thomas mention this in the Pragmatic Programmer. They say something like "you've got plenty of experience, so don't ignore that nagging feeling" (if you have the right quote, or if it's from another book, please correct me).

Carl