Analyzing the code base of CruiseControl.NET

 

As I already did with
several popular.NET projects like NUnit,
NHibernate,
.NET Framework,
Silverlight,
I found interesting to see what we can say about their architecture, structure and quality.
To do so, I use the static analyzer NDepend
and sometime I have the chance to debate with some developers of these
products. Today, I would like to focus on the CruiseControl.NET
code base.
CruiseControl.NET is an Automated Continuous Integration
server, implemented using the Microsoft .NET Framework. It is developed by the
company ThoughtWorks
.

 

With 13 152 lines of
C# code
,
CC.NET is a relatively small code base.
Here are the global key metrics
obtained from the CC.NET code base :

 

# IL instructions    86 168

# lines of code
(LOC)    13 152

# lines of comment    5 996

Percentage Comment    31%

# Assemblies    8

# Namespaces    52

# Types    634

# Methods    4 030

# Fields    1 867

 

Coverage:

Percentage Coverage    37%

# Lines of Code
Covered    4 944

# Lines of Code Not
Covered    8 208

 

Tier code used by the application:

# Tier Assemblies
used    14

# Namespaces used    57

# Types used    442

# Methods used    1 235

# Fields used    72

 

 

Test Coverage of CruiseControl.NET

 

It comes with 919 unit
tests, on which 17 are failing when I open the main VS solution, rebuild all,
and start testing with TestDriven.NET.
37% of the code base is covered which is not so much. However a lot of effort
has been put in unit testing considering that the test assembly ThoughtWorks.CruiseControl.UnitTests
weights 13 717 lines of code alone, more than the code base tested. Also,
we can see that the tests are focused on some particular parts of the code. When
a part of the code is tested, it is more than 80% covered.

 

On the NDepend Metric view below, rectangles represents methods and the size of a rectangle is proportional to the number of lines of code of the underlying method. Rectangles methods are hierarchized by types, namespaces and assemblies. Blue rectangles represents methods at least 80% covered, as indicated by the CQL Query below.

 

 

Architecture of CruiseControl.NET

 

The bulk of the code is
spawned on 3 assemblies: Core, CCTrayLib and WebDashboard.

 

SELECT ASSEMBLIES WHERE !IsFrameworkAssembly ORDER BY NbLinesOfCode DESC 

assemblies

# lines of code (LOC)

ThoughtWorks.CruiseControl.Core

7480

ThoughtWorks.CruiseControl.CCTrayLib

3629

ThoughtWorks.CruiseControl.WebDashboard

1591

Objection

189

ThoughtWorks.CruiseControl.Remote

177

ccservice

56

cctray

25

ccnet

5

 

Here is the dependency
graph of assemblies:

 

 

The code base is nicely partitioned
through assemblies
,
with few assemblies, but larger. As often in such situation, the downside is that the
code in each big assembly is entangled. Here are the namespaces of CC.NET seen from the NDepend
dependency matrix. There are 2 massive dependency cycles
(represented with red square):


 

 

Here what looks like such
a cycle. Clearly the namespace ThoughtWorks.CruiseControl.WebDashboard.Dashboard
is pretty entangled and some important refactoring is needed here:

 


 

And here is a focus on a
cycle involving two namespaces. These kind of focus are good helpers to let the
team decide where the cycle should be cut. Here, the first thing to do is to
remove double-arrows (in red) and then decide where the cycle should be cut by using some abstractions.

 

 

To obtain such focus on a
cycle involving two namespaces from VisualNDepend, the easiest way is to set
the indirect dependencies mode in the
dependency matrix and to click on the
cell corresponding to the 2 namespaces. The weight  of the black cell indicates here a dependency
cycle with a minimal length 7:

 


 

 

Bad usage of Copy Local Reference Assembly option
set to True

 

Unfortunately, as in most .NET code bases, CC.Net VS projects rely on the copy
local
reference assembly option set
to true.

 

 

While dissecting the
NUnit code base
I demonstrated that setting
this option to true is a bad thing. It is easy to see that assemblies get over-duplicated
by doing a search on the root path.

 

 

Not only this increase significantly
the compilation time (x3 in the case of NUnit), but also it messes up your
working environment. Last but not least, doing so introduces the risk for versioning potential problems. Btw, NDepend will emit a
warning if it founds 2 assemblies in 2 different directories with the same
name, but not the same content or version.

 

The right thing to do is
to define 2 directories $RootDir$\bin\Debug
and $RootDir$\bin\Release, and configure
your VisualStudio projects to emit assemblies in these directories. All project
references should reference assemblies in the Debug directory. As a bonus, you have the possibility to emit your
tests assemblies in the $RootDir$\bin
directory. This way, with the astute of assembly config files, applications
assemblies can be tested easily. Such a config file can look like this (you can add
as many relative sub dir as needed through the privatePath attribute):

 

By setting this option to
true by default, Microsoft fosters bad practices on the usage of Visual Studio.

 

 

CruiseControl Quality

 

The CC.NET code base
overall quality is pretty good considering classical metrics. Only 75 methods
on 4 030
slightly exceed
thresholds
(the report tells 5 265 methods because it includes tier code
methods).

 

WARN IF Count > 0 IN SELECT METHODS WHERE 
!NameIs "InitializeComponent()" AND
                                           
// Metrics' definitions
     (  NbLinesOfCode > 30 OR              // http://www.ndepend.com/Metrics.aspx#NbLinesOfCode
        NbILInstructions > 200 OR          // http://www.ndepend.com/Metrics.aspx#NbILInstructions
        CyclomaticComplexity > 20 OR       // http://www.ndepend.com/Metrics.aspx#CC
        ILCyclomaticComplexity > 50 OR     // http://www.ndepend.com/Metrics.aspx#ILCC
        ILNestingDepth > 4 OR              // http://www.ndepend.com/Metrics.aspx#ILNestingDepth
        NbParameters > 5 OR                // http://www.ndepend.com/Metrics.aspx#NbParameters
        NbVariables > 8  )                 // http://www.ndepend.com/Metrics.aspx#NbVariables
       

 

 

This entry was posted in Uncategorized. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://ferventcoder.com Robz

    “By setting this [copy local reference assembly] option to true by default, Microsoft fosters bad practices on the usage of Visual Studio.”

    Actually, if you look at the project file in Notepad, you will see that there is no default setting. It’s not true by default. It’s not false either. We’ve seen a bug where when building the source code with MSBuild, it forgets to include referenced assemblies on build servers due to exactly this issue. Then when you deploy you are missing those assemblies.

    http://ferventcoder.com/archive/2008/08/19/tfs-2005-team-build-error-referenced-assembly-missing-not-in.aspx

    [It's not TFS Team Build with the issue like I originally thought when I posted the article].

    This is what it looks like when VS says copy local is set to true (when it’s not set to anything).

    False
    ..\ThirdParty\log4net.dll

    And this is what it looks like if it is really set to true:


    False
    ..\ThirdParty\log4net.dll

    True

  • http://www.NDepend.com Patrick Smacchia

    Igor, that is an intersting point you are underlying indeed.
    The choices to store shared core DLLs between the different presentation layers are:

    you create a folder for each presentation layer and core shared DLL are duplicated in each one.

    you make it so that there is a folder that contains core shared DLL, and presentation assemblies can have access to this folder to load core shared DLL.

    you can also store shared DLLs in the GAC

  • http://igorbrejc.net Igor Brejc

    Patrick,

    You fail to mention the fact that there are multiple targets which are produced as part of the CC.NET solution: Web application, Win. service, console app. and a Windows desktop app. Are you sure putting everything into a single target directory is a good policy? Why would you need Web app. dll mixing together with the one for the Windows service, if they are not really dependent upon each other?

  • http://www.NDepend.com Patrick Smacchia

    Craig, Dave, I feel honored that you that you consider revisiting mentionned issues.

    Let me know if the NDepend team can be of any help for further analysis of next CC.NET versions.

  • http://csut017.wordpress.com/ Craig Sutherland

    Thanks Patrick for the analysis, it’s very helpful to see what areas we need to improve in CruiseControl.Net.

    We’ll take your analysis on board in planning for the next version.

    Craig

  • Dave Cameron (CCNet maintainer)

    The problems with Copy Local are a very good point. I have always used the approach you describe for command-line builds and CCNet’s command-line build takes this approach too.

    But I have never changed the project files so that VisualStudio would do it too. I can’t really think of a good reason why not… I guess just thinking “Microsoft knows best.”