High .NET Code Maintainability is the key to achieve both happy management and happy developers:
- Maintainability lets a product evolve naturally at a sustained pace with controlled cost.
- Maintainability lets developers add new features and improve existing ones without spending most of their time refactoring old dusty code and fixing bugs.
After 20 years of development on our product NDepend (first release in April 2004!) we came to the conclusion that:
Highly .NET Maintainable Code can be achieved through two simple, objective and verifiable principles: Layered Architecture and High Test Coverage Ratio
Layered Architecture prevents entangled code, the well-known spaghetti code phenomenon. It fosters full control over dependencies. When it is time for the code to evolve new classes and interfaces naturally integrate with existing ones.
High Test Coverage Ratio means that when code covered by tests gets refactored, existing tests get impacted. With not much effort the developer discovers regression problems and fixes them before they go to production and become bugs to fix. The more code is covered by tests the more you’ll benefit from this shield.
When writing a tool for developers, the most satisfying part is to challenge the tool on its own code: this practice is named dogfooding. We just rewrote completely the dependency graph of NDepend so let’s use this important refactoring as a case study. Then we’ll see how we continuously validate these principles.
Case Study: Layered Architecture
Let’s first present the layered architecture principle and then the test coverage principle.
See below a graph of the 250+ classes, interfaces, and enumerations used to implement the new dependency graph. A 2.500+ classes, methods and fields SVG vector dependency graph is available here.
The class GraphController is selected:
- The blue classes are the ones directly used by GraphController
- The light-blue classes are the ones indirectly used by GraphController (indirectly means used by classes used by a class … used by GraphController). Clearly, GraphController relies on everything.
- The red classes are the ones mutually dependent on GraphController.
Remarks about Layered Architecture
Here are some remarks about this code structure:
- This is not an API so we can use namespaces the way we want. Here namespaces implement the concept of components.
- Box size is proportional to the number of lines of code. We can see that the overall namespaces box size is well-balanced. This is a good practice to avoid having a few monster components and tons of smaller components.
- The biggest component in terms of number of classes and lines of code is the implementation of the Undo/Redo system. We developed more than 30 actions (expand/collapse, change GroupBy, select/unselect, generate a call graph…). These actions are relatively low level in the structure. While they act on the entire system they are not coupled with the controller, the UI rendering, or the layout computation.
- The two lowest components are Base and Model. Both contain few logic and are used by almost all other components.
Why does it achieve High Maintainability?
In the future, whether we add new actions to the graph or decide to improve the layout somehow, this architecture won’t undergo drastic modifications. Thanks to this view it’ll be easy to decide in which component to add our new classes or if new components should be added and what they can and cannot use.
Ideally, the GraphControl class shouldn’t be entangled with the GraphController class. These two classes have been developed together. See below the coupling graph between GraphController and GraphControl. It has been obtained by double-clicking the red arrow between the two classes. It wouldn’t be difficult to introduce an interface to inject one implementation into the other one but we didn’t do it (see below the coupling graph between the two classes). This is the key when it comes to caring for maintainability: which move will offer the highest ROI? Not everything has to be perfect just for the sake of it. Experience shows that having only two classes entangled does not impact maintainability. We estimated that spending our resources to satisfy the two principles has a better ROI in the long run.
Case Study: High Test Coverage Ratio
The graph implementation is 90% covered by tests. It is not because there is a lot of UI code that it should not be well-tested. We didn’t spend a good part of our resources on writing tests just for the sake of it. We did it because we know by experience that it’ll pay off: probably a few bugs will be reported for every 1.0 implementation although beta test phases already caught some. But we are confident that it won’t take a lot of resources to fix them. We can look forward to the future confidently.
Visualizing High Test Coverage Ratio
The picture below shows all namespaces, classes, and methods. Smaller rectangles are methods and the color of each rectangle indicates how well a method is covered by tests. Clearly, we tolerate some gaps in UI code, while non-UI code like Undo/Redo actions implementations are 100% covered. Here also experience tells us how to balance our resources and that everything does not have to be perfect to achieve high maintainability.
In terms of lines of code, the NDepend Graph is not even 5% of the entire product, it is a tool in the toolset. The worst case scenario would be that each tool implementation regularly spits some bugs: all our resources would be spent fixing them, we couldn’t continue adding value to the product and the business would probably die at a point. Not even mention the frustration of users of a buggy product.
Each year we fix a few dozen of bugs that each impact a few users but that doesn’t take us more than a tiny percentage of our overall development resources. The overall code base is 87.2% covered by tests and is entirely layered: maintenance doesn’t cost us much.
Typically at this point comes the remark: but code coverage is not enough, unit tests must assert results. And as a matter of fact, with no assertions tests are dysfunctional even if the code is entirely covered by tests. We want tests to fail when something is going wrong. In this next post Case Study: Complex UI Testing I explain how millions of assertions get checked while running our test suite against the graph implementation. Watching the test execution turns to be mesmerizing.
Continuously Validating Layered Architecture and High Test Coverage Ratio
NDepend offers hundreds of default code rules. However, only 4 of them are validating these key points:
- Avoid namespaces dependency cycles
- New Methods should be tested (new methods introduced since the baseline)
- Methods refactored should be tested (here also, methods refactored since the baseline)
The fourth rule Avoid namespaces mutually dependent helps a lot to layer a large super-component. In this situation, the first thing to do is to make sure there is no pair of components that use each other. For each such pair of namespaces matched, this rule has a heuristic and tells which type should not use which other type, same for the method level. The rule proposes a technical-debt estimation in terms of the development effort cost to fix each pair.
Here it says that the team should spend 11 man-days (8 hours a day) if someone decides to layer the NHibernate code base. Unfortunately, this is not possible because it would break thousands of client code bases bound with it. Also, let’s note that an interest estimation is also given in terms of how much development effort it takes per year if I leave issues unfixed. Here this rule estimates that not fixing all those pairs of namespaces entangled costs 5 man-days a year to the development team.
These rules can be validated during the build process (Azure DevOps / TFS, Jenkins, TeamCity, Bamboo, SonarQube…) and the team can know when the new code written diverges from these two maintainability goals.
Conclusion: Objective, Verifiable, Simple
What is interesting about these two simple concepts, layering and code coverage, is that they can be objectively applied, validated, and measured. Recently I wrote a blog post series on SOLID principles and there has been so much debate about how to apply them in the real world. SOLID principles are a great way to improve our understanding of Object Oriented Programming and how encapsulation, abstraction, polymorphism, inheritance … should be used and not used. But when it comes to writing maintainable code everyone has a different opinion.
If it is decided that the code structure should be layered there is not much debate about which part should be abstracted from other ones. If a class A should use a class B and B is in a higher layer than A, somehow an interface IB must be created at A level. This way we can inject the implementation of B in A without breaking the layering.
These 2 concepts emerged over the years because we had the utter need to produce maintainable code. What I really like is that they are simple. And KISS (Keep It Simple Stupid) is a great principle in software engineering.
If a third principle should be added it would definitely be about user documentation: we offer free email support to users but we also offer tons of embedded and online documentation. Every time a question starts to be asked a few times, we make sure that users can get the response immediately from both a tooltip (or a smart UI change) and from the online documentation. Some other ISV decide to make money with support. Personally, I don’t find this fair. Doing so constitutes an incentive to produce rotted documentation and hence friction for the user.
How did we obtain the image in this post
Let’s show that all those images in this post have been obtained within a few clicks.
- First, let’s search for Graph Panel in the entire NDepend code base (they get zoomed automatically).
- Then let’s reset the metric view with NDepend.UI.Graph.* namespaces to get the colored treemap.
- Then let’s go back to the graph and only keep NDepend.UI.Graph.* namespaces matched by the search.
- Then un-group by parent assembly to get a graph made of namespaces only.
- Then change the layout direction from Top to Bottom to have a nicer layout.
- Then expand all namespaces to get all classes.
- Finally, expand all classes to get all methods and fields.
