Evaluating NDepend on current codebase Part 2

In Part 1 we saw a first approach to NDepend starting off with analyzing a project of 2 assemblies and focusing on the smaller assembly. In this part we will focus on the main executable and the gems NDepend discovered in it TL; DR Memory leak detected...


Photo by Daan Mooij on Unsplash

Enter 3 years old WPF project - Main executable

Going back to the dashboard I see the "usual" long types or methods errors, some immutability warnings, which I would have set as errors and 400 violations of "Avoid namespaces mutually dependent" some with debt time of 4 days. "Oh come on.. what 4 days. How hard can it be..."

Drilling down into the code (a wpf application) I realize that there is a single class that is injected in the constructor of every view model and I hardly see any services in the system save for calls to the DAL assembly.

Without drilling down into the class I changed it's namespace and made the necessary, albeit sisyphic, changes to all the classes. While at it I noticed that some view models where creating instances of view models for dialog boxes, panes and popups and injected the Container class to them as well. 

Once the change was completed, I run NDepend on the result and I had 800 violations of the namespace rule plus some API breaks. Oh my...

A lesson learned: Always drill down before making such change

The injected class, let's call it Container, has 2 properties pertaining to 2 classes, Let's call them Singletons and Factories. 

A first look reveals nothing suspicious; Singletons holds services and Factories creates view models when needed. Sounds reasonable right? Wrong... To quote myself:
there is a single class that is injected in the constructor of every view model 
In fact, every time the Factories class creates a class it injects in its constructor... you guessed it, the Container class. Moreover, elements in the Singleton class created view models via the Factories and these view models in turn require use of these same (or other) singletons. In fact, some view models also serve as service classes and there is more than one way to create a view model; By factory or by directly initializing via constructor. Here is a diagram of the dependencies between these classes.

There really aren't enough lines to describe this

When there is a "God" obejct in any system it almost always ends up looking like this. Sometime weird performance issues are detected in the system and people tend to use GC.Collect to deal with them and blame some 3rd party black box or may some obscure Microsoft bug, not that this isn't known to happen. However when some GC.Collect calls seem to solve the problem that means that there is a memory leak and a misbehaving object ruining loose.

Going forward with the D in S.O.L.I.D

A lot of time will be spent fixing this one obviously, mainly by introducing a proper dependency injector and IOC container. And in every step of the way this code as well as any new code in the development team will have to be analyzed by NDepend to catch problems on time. The build engine integration of NDepend should be of great value to catch code smells as they are committed. 

Apart from the features portrayed in these posts NDepend is feature rich, including code coverage files (which I have yet to test), trends, metrics and graphs to show you your project analysis from any view and angle possible. I am hooked. How do you perform your static code analysis? Leave a note in the comments below.

Comments

  1. In those moments when it seems that the problem is being solved very quickly, you should not rejoice as this indicates a data leak and even bigger problems.

    ReplyDelete
    Replies
    1. I most definitely agree. It was too easy to be true. When I actually fathomed the graph of dependencies I had a face-palm moment.

      Delete
  2. I also agree with the previous commentary. What's more, our evaluation is trustworthy and reliable, so we can easily perform your static code analysis without any worries.

    ReplyDelete

Post a Comment

Keep it clean, professional and constructive. Everything else including ethnic or religious references will be removed.

Popular posts from this blog

NUnit Console Runner and NLog extension logging for your tests

Tests code coverage in Visual Studio Code with C# and .Net Core

Applying an internal forums system in the company culture - thoughts