NDepend to the Rescue – Again

The last few years I’ve been working in Java, Kotlin, Ruby, Flutter and Delphi, but I’ve finally had the opportunity to return to .NET programming and my favourite language C#. I’ve been working feverishly on a WinForms project, but despite efforts to write clean code, I’ve taken shortcuts which I need to address before moving forwards.

One of the benefits of .NET programming is the strong static typing and rich assembly metadata, which enable tools to provide high quality code analysis and feedback. NDepend is one such tool.

As mentioned in a previous post, I am a huge fan of NDepend. Please see that previous post for an introduction to NDepend. You can also view these video walk throughs. Here, I discuss how it has helped me address technical debt in my current project.

Are You Sure About That?

Straight off the bat, I’m arguing with it. The Hub Messaging assembly only contains concrete message types by design, just like a Protobuff file contains messages. What’s wrong with that?

Let’s look closer at the rule:

I absolutely love the feedback. Whether you agree or disagree, it is thought provoking.

Okay, I’m starting to argue with myself. It may become useful in the future to subscribe to a category of messages, in which case I would subscribe to an interface not a concrete class, or even a class and its descendants. I can see where the zone of pain makes sense.

Currently an observer can subscribe to many messages, so it’s not the end of the world. But the rule is correct, it is a pain to have to add the abstraction at this stage, and apply it across a number of assemblies. And if I don’t do it now, it will only get more painful as the number of messages grow. The further it drifts from centre, the more painful to refactor.

The question is, do I need that abstraction? In hindsight, I’d liked to have thought about this from the beginning, if I’d only been aware of the issue.

Another decision I had made early was to subscribe/dispatch against a type string name due to problems processing messages from on-the-fly compiled IronRuby scripts. If I’d used NDepend from the start, I’d have noticed this assembly’s gradual drift from the centre, as per the diagram, and would have had this conversation a lot earlier. There are ways around this issue if it becomes a problem, for example I could introduce envelope like messages. But still, NDepend is asking the right questions. Do I need that abstraction? If so, fix it now before it becomes even more painful.

Paolo Cantoni, a mentor and good friend, often advised when designing: role play. Advocate a design on the white board, physically change positions, then challenge your design. Think from different perspectives, move from thesis to antithesis to synthesis in an Hegelian dialog with yourself. What I love about NDepend is that it is that adversary. It challenges you, it invokes these kinds of conversations even on an architectural level. You may disagree with it, but it provokes such considerations. That makes it invaluable to me, that’s what I like best about NDepend.

Regarding the other two classes slipping from centre, I’m not too worried. Hub.Infrastructure is literally a collection of utility classes, and Hub.Domain is pretty bare bones at the moment. But I will be keeping an eye on them. The four core assemblies are sitting pretty.

Cyclomatic Complexity Looking Good

The TreeMap Diagram looks pretty healthy, with one of the biggest offenders being the InitializeComponent method which is autogenerated, so I’m not too worried about that.

I did reach out to the team about ignoring autogenerated code, and they kindly replied: “Yes with notmycode queries, normally Winforms designer method InitializeComponent() gets ignored”:

I also use the Monaco Editor in several forms, so some cyclomatic complexity is unavoidable:

 private void SaveButton_Click(object sender, EventArgs e)
{
    GetContents().ContinueWith(task =>
    {
        var text = task.Result;

        if (_content != text)
        {
             SaveFile(text);
        }
    });
}

In fact most interactions with CEFSharp, WebView2, and Monaco are asynchronous. The IDE itself is a TCP/IP server which captures log information, so in a number of places there is thread/task management, which is by nature complex. And then there’s management of, and communication with, docked external processes, such as the Ubuntu shell, or Chrome Design Tools etc.

I’m pretty happy with the code at this stage.

Are You Sure About That?

Again, NDepend has provoked a conversation. Hub.App assembly used to look like this:

Considering feedback from NDepend, after refactoring, it now looks like this:

What a difference, that is much cleaner!

By vertically organizing by feature, rather than type, I am encouraged to extract inner classes, enumerations, and even methods to their own file. FindService is now much more simple, with responsibilities (such as the report) pushed to other classes. The grouping is pragmatic, and neat.

As illustrated by this NDepend graph, architecturally I think I’m finally in a decent place. There are a few issues, but in general, it’s okay. The dependency graph is interactive.

Here we can see that the thickness of the arrow signifies the amount of coupling:

What’s Next?

Now it’s time to work through the more granular issues reported by NDepend which are spread across multiple categories, such as Code Smells, Object Oriented Design, Immutability, etc.

NDepend allows you to attack these issues via multiple methods, the most obvious is by double clicking on an issue which leads a detailed view of the broken rules:

I often prefer to attack the problems one assembly, one class at a time:

A particular issue NDepend pointed out was the mutual dependence between types I had in the same assembly. I thought nothing of this – there was no compilation problems, and my assertion classes could take advantage of the string and collection extension classes, and vice versa:

But NDepend had this to say:

It also provides a useful matrix to highlight these dependencies.

After some consideration, I concur with NDepend. Even within the same assembly I would like, as much as possible, for dependencies between types to be unidirectional. It is just good programming basics: high cohesion, low coupling. Interfaces are a way to solve this issue, but there’s little need for abstraction in these fundamental utility classes.

The solution in the above example was simple: the integrity classes no longer use the collection extensions. I have a few more such dependencies to weed out, with NDepend’s help, but moving forwards I’ll be conscious to avoid such issues. I might be getting old, or it may be I was mostly coding alone and tired, but it’s easy to overlook these things at times. That’s why it’s great to have a tool like NDepend looking over your shoulder.

I Beg to Differ

Sometimes you may disagree with NDepend. For example, Microsoft has deprecated the rule AvoidNamespacesWithFewTypes, I also disagree with this rule. I can see the benefits to the refactored Hub.App mentioned above, which is now in good order. Still, in some cases it may be a problem, so it is right that NDepend raises a warning. Suppressing the message is quite simple:

1. First, create a file in your project called GlobalSuppressions.cs.
2. Next, right click on a broken rule, and select Supress Rule.
3. NDepend will show a dialog with the code necessary to supress the rule
4. Within your suppressions file, paste the code.

This is what the dialog looks like:

…and in the case of the AvoidNamespacesWithFewTypes my suppresions file looks like this:

using System.Diagnostics.CodeAnalysis;

[assembly: SuppressMessage("NDepend", "ND1305:AvoidNamespacesWithFewTypes", Target = "",
    Scope = "module namespace type method field", Justification = "Suitable for project requirements")]

I must say, however, that even when I differ with a rule, I learn something:

Communication

Another fantastic feature of NDepend is its ability to surface work.

Often it’s difficult for management to understand just how much work is involved in refactoring a code base, addressing the technical debt, and making sure the project is healthy. Developers often find it difficult to articulate what they have been doing for the last few days, there’s no new functionality, just constant refactoring. Despite our best efforts, the code is often not as clean as we think it is, and subtle issues are taking root which may hamper future development velocity. NDepend does a great job of communicating this technical debt:

NDepend makes it easy to communicate not only what needs to be fixed, but comparing against a baseline, it can report on progress and provide up-to-date estimates based on the number of outstanding issues and the number of new issues raised.

In fact, once your project is in order, moving forwards you could make the case for putting aside the last day of each sprint for the team just to attack the technical debt NDepend is identifying.

Summary

Testing catches fewer bugs per hour than human inspection of code. Inspections are a cheaper method of finding bugs than testing. If you want to ship high quality code, you should invest in more than one of formal code review, design inspection, testing, and quality assurance.

NDepend delivers so much of this and more, I can’t recommend it highly enough.

The earlier it is introduced to the project, the better.

I hope you may have found this post helpful.

Advertisement