Do a favor to yourself
and your team: Make sure that the code you will develop in 2008 will abide by common
quality principles.
The classic blame on
quality tools I hear again and again is: When the tool analyzes my code base,
it yields literally thousands of warnings. I don’t have the time to go through
all these problems.
Actually, this blame
is similar to the one that is made against automatic tests. I don’t have the
time to stop my development for months just to code unit tests on existing code.
There is one simple answer to this situation apparently widely accepted amongst
the agile experts: from now, make sure
that all the code you are doing or refactoring will be unit tested.
I propose the same
answer for code quality: from now, make
sure that all the code you are doing or refactoring will abide by common
quality principles. And now comes the tricky part: how can I specify to my
quality tool which part of my code base is new or has been refactored?
NDepend and the language CQL are able to compare 2 versions of
a code base. The idea is then to regularly check that the quality will be respected
on the code that has been changed since today. Concretely, you just have to
write:
WARN IF Count> 0 IN SELECT METHODS WHERE
(CodeWasChanged OR WasAdded) AND CyclomaticComplexity > 15
The conditions CodeWasChanged OR WasAdded can be understood as Code that
has been refactored or new code.
The NDepend project
can be easily configured to define with which previous analysis the comparison
will be made against. The following screenshot shows how to specify a
particular analysis, made on the 1st January 2008. You can also compare
against an analysis made N days ago or the last analysis done.
Here are 10
essential quality rules I would suggest. The great news is that this tips can be readily applied to any CQL rules!
//
<Name>Methods too complex (CyclomaticComplexity)</Name>
// A method should not be too big.
WARN IF Count> 0 IN SELECT METHODS WHERE
(CodeWasChanged OR WasAdded) AND NbLinesOfCode > 25
//
<Name>Methods too complex (CyclomaticComplexity)</Name>
// A method should not be too complex.
WARN IF Count> 0 IN SELECT METHODS WHERE
(CodeWasChanged OR WasAdded) AND CyclomaticComplexity > 15
//
<Name>Methods poorly commented</Name>
// A method with at least 10 lines should be commented.
WARN IF Count> 0 IN SELECT METHODS WHERE
(CodeWasChanged OR WasAdded) AND
PercentageComment < 20 AND NbLinesOfCode > 10
// <Name>Methods
with too many parameters</Name>
// Methods with many parameters are hard to maintain
and are bad for perf.
WARN IF Count> 0 IN SELECT METHODS WHERE
(CodeWasChanged OR WasAdded) AND NbParameters > 5
// <Name>Methods
with too many local variables</Name>
// Methods with many local variables are hard to
maintain.
WARN IF Count> 0 IN SELECT METHODS WHERE
(CodeWasChanged OR WasAdded) AND NbVariables > 7
// <Name>Inheritance tree too
deep</Name>
// Branches too long in the derivation should be
avoided.
WARN IF Count > 0 IN TYPES WHERE
(CodeWasChanged OR WasAdded) AND DepthOfInheritance > 5
// <Name>Types with too many fields</Name>
// Types with too many fields should be avoided.
WARN IF Count > 0 IN TYPES WHERE
(CodeWasChanged OR WasAdded) AND NbFields > 10
// <Name>Methods
that use boxing</Name>
// Boxing\Unboxing should be avoided for performance
reasons.
WARN IF Count> 0 IN SELECT TYPES WHERE
(CodeWasChanged OR WasAdded) AND
(IsUsingBoxing OR IsUsingUnboxing)
// <Name>Methods
that could be better encapsulated</Name>
WARN IF Count> 0 IN SELECT METHODS WHERE
(CodeWasChanged OR WasAdded) AND
(CouldBePrivate OR CouldBeInternal OR CouldBeProtected OR CouldBeInternalProtected)
// <Name>Types
that could be better encapsulated</Name>
WARN IF Count> 0 IN SELECT TYPES WHERE
(CodeWasChanged OR WasAdded) AND
(CouldBePrivate OR CouldBeInternal OR CouldBeProtected OR CouldBeInternalProtected)