My post .NET Framework 3.5 SP1: Changes Overview on analysis evolution, structure and quality of the .NET framework code base with NDepend became popular. It shows the interest of the community for the under the hood of popular Fx. Thus came the idea of publishing a similar post for the new release of NHibernate 2.0. This post NHibernate 2.0 Gold Release analysis NHibernate 2.0 with NDepend and lists breaking functional changes, while I'll focus more on structural changes.
Changes Overview (compared to NHibernate v1.2.1)
# IL instructions 176 818 to 245 106 (+68 288 +38.6%)
# lines of code (LOC) 25 960 to 36 143 (+10 183 +39.2%)
# lines of comment 25 135 to 29 401 (+4 266 +17%)
Percentage Comment 49% to 44% (-5%)
# Assemblies 1
# Namespaces 46 to 65 (+19 +41.3%)
# Types 806 to 1 262 (+456 +56.6%)
# Methods 8 190 to 12 016 (+3 826 +46.7%)
# Fields 2 166 to 3 842 (+1 676 +77.4%)
4.852 new
public methods:
SELECT METHODS WHERE IsPublic AND WasAdded
548 new
public types:
SELECT TYPES WHERE IsPublic AND WasAdded
21 new mamespaces:
SELECT NAMESPACES WHERE WasAdded
2 namespaces removed:
SELECT NAMESPACES WHERE WasRemoved
127 public types removed:
SELECT TYPES WHERE IsPublic AND WasRemoved
5 non-public methods became public:
SELECT METHODS WHERE IsPublic AND VisibilityWasChanged AND IsInNewerBuild
1.230 methods
where code was changed:
SELECT METHODS WHERE CodeWasChanged
380 types
where code was changed:
SELECT TYPES WHERE CodeWasChanged
The following treemap/metric view shows the impact of methods changes or added (in blue). It clearly looks like that the entire code base has been refactored:
Assembly dependencies
The assembly structure hasn't changed. The NHibernate code is still packaged within one assembly named NHibernate.dll, and this is IMHO the best possible choice since there is no reason to have several physical components for the NHibernate Fx.
NHibernate.dll is almost using the same set assemblies: Castle.Core.dll v1.0.3.0 has been added and also Castle.DynamicProxy2.dll v2.0.3.0 is used in lieu of Castle.DynamicProxy.dll v1.1.5.0. Here is the simple graph of assemblies dependencies (made with the version of NDepend v2.10 under development, so stay tuned for this future exciting release scheduled in a few weeks).
Internal namespaces dependencies
As noted by Jim Bolla a few months ago in his post Analyzing NHibernate with NDepend, the NHibernate code is quite entangled and unfortunately this problem hasn't been fixed for NHibernate v2.0. Basically, on 65 namespaces, 63 namespaces depend directly or indirectly on 62 other namespaces. This is what is shown by the big black square in the dependencies matrix below, taken with the indirect dependencies option of NDepend. A black cell means that the 2 corresponding namespaces are involved in a dependency cycle of minimal length the number displayed.
IMHO, the NHibernate team should fix this problem asap. This article Controlling Dependencies to get Clear Architecture explains how we get rid for good of spaghetti in the NDepend code base within in a few days, about 2 years ago. Retrospectively, I estimate that this is by far the best Return On Investment on quality we ever done, much better that any of the thousands automatic tests we wrote. As explained in the article, we now use some NDepend capabilities to prevent new cycle to appear.
The picture below shows direct dependencies between namespaces of NHibernate. The legend is:
- A blue cell means: {the X Namespace} is using {the Y Namespace}.
- Weight of a blue cell means: W members (methods and fields) of the {the X Namespace} are used by {the Y Namespace}.
- A green cell means: {the Y Namespace} is used by {the X Namespace}.
- Weight of a green cell means: W methods of the {the Y Namespace} are using {the X Namespace}.
- A black cell means: {the X Namespace} and {the Y Namespace} are using each others.
- A red tick on a cell means: the coupling has been changed.
- A red tick with a plus on a cell means: the dependency has been created.
- A red tick with a minus on a cell means: the dependency has been removed.
- A Namespace name underlined means that its code has been changed.
- A Namespace name in bold means that it has been added.
Code quality
According to the following CQL rules, 464 methods are good candidate to be refactored because they each violates one of the metric threshold.
// <Name>Quick summary of methods to refactor</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE
// 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 OR // http://www.ndepend.com/Metrics.aspx#NbVariables
NbOverloads > 6 ) // http://www.ndepend.com/Metrics.aspx#NbOverloads
This stance on metrics is a bit extremist and honestly, we have a bunch of methods in the NDepend code base that still don't abide by all these thresholds. While abiding by metric threshold helps a lot to have maintainable code, my opinion is that quality effort must be focused first on rationalizing componentization and dependencies, with non-cyclic dependencies between components, low level components and logical component (namespaces) instead of physical components (assemblies).
However, there is a fundamental metric not represented here: Test Coverage. I tried to gather Test Coverage metrics for NHibernate 2.0 to make this data analyzed by NDepend. Unfortunately just running the NHibernate.Test-2.0 project with the option Run Test(s) of TestDriven.NET lead to: 427 passed, 755 failed, 14 skipped, took 3738,55 seconds. I suspect I don't have all DB stuff installed (I am not really a DB guy) and am a bit lazy to go thought all the operation to make it work. (Btw, any code coverage file produced by NCover or VSTS Coverage on NHibernate 2.0 is welcome, just send it with http://www.transferbigfiles.com/ and let me know the url where I can download it. My email is psmacchia at google mail. This way I could complete this post with obtained results).
Conclusion
This awesome release of NHibernate is the result of a lot of work done by extremely talented guys. As said, I am not a DB guy, I cannot appreciate the real value of this framework. However, googling it a few seconds lets find dozens of thousands of enthusiast users.
This post focuses on structure, quality and evolution and thanks to NDepend capabilities, I find some good things but also some important room for improvement (IMHO) within a few minutes that I wanted to share.