Profile photo of Anton

by

Code Deodorant

October 28, 2013 in .NET, Elements, Tools, Visual Studio

We all write smelly code from time to time. Reasons are different, from ‘I’ll rewrite this code later, I just need this feature working now somehow’ to ‘Deadline is coming and this pesky bug is still not fixed’ or some mistake from a junior developer, yet the result is always the same. Every large codebase has lines of code that smell and stink. Of course there is an ideal codebase repository somewhere that does not have a single line of dirty code, or odd reinvent-the-wheel solutions, or patterns misuse etc… But, I’m afraid, that is along the lines of a flying unicorn. In the real world, we sometimes have to sacrifice code purity to deadlines or other reasons. In this case the main goal is to keep the unclean code and the technical debt caused by it manageable. Not a simple task, when the project has more than 55k lines of code, like ROFX/.NET (i.e. Data Abstract, RemObjects SDK, Internet Pack and Script for .NET) does.

So we need a tool that analyzes our source code and helps to pinpoint code that is potentially dirty. Such tools are called Static Code Analyzers. There are several of them available for .NET and I’d like to highlight the one I like best. This particular tool is not tied to C# or VB.NET and works perfectly with Oxygene code. It also provides Visual Studio integration (including the upcoming VS 2013), and it can also be run on a build machine as a part of the build process. This awesome tool is called NDepend (http://www.ndepend.com/).

In the following, we will create a simple application and apply NDepend to it. We’ll use Oxygene, as I need to show you how to avoid some sharp edges. While C# support in NDepend is near perfect, there are some (minor) nuisances that need to be avoided for Oxygene projects.

Let’s create an Oxygene for .NET Class Library project and add some code like this:

namespace SampleClassLibrary;

interface

type
  Class1 = public class
  private
    var fUnusedField: Int32;

    method NeverCalledMethod(); empty;
  public
    constructor(); empty;
  end;


  UtilityClass = public class
  public
    class method Foo(); empty;
  end;


 SomeBaseClass = abstract class
  public
    constructor(); empty;

    method AbstractMethod(); abstract;
  end;


implementation
end.

As you can see, there are several things that are not ideal, like unused fields and methods or the public constructor of an abstract class. This is, of course, simplified code, where all such things are clearly visible. At the same time, it is very hard to find all suspicious places in a real-world codebase, where the code was created by dozens of different developers and is spread between several projects containing several hundreds of code files. Of course thorough code review would help, but it is still a question of how much efforts it would require. And the question that remains – even after the code review – is to be sure that everything is clean?.

Enter NDepend.

Attach NDepend to the current solution using the menu item NDepend -> Attach new NDepend project to current VS solution.

Because NDepend doesn’t recognize Oxygene projects out of the box, we need to reference the compiled assemblies instead of the source code project, as we would do for C# or VB.NET projects. Press the ‘Add assemblies in Folder‘ button and select the folder containing the current project’s compiled assemblies. Note that NDepend will automatically preselect the folder containing compiled assemblies for the active project.

Confirm the folder selection. Assemblies will be added to the NDepend project.

Press the ‘Analyze a single .NET assembly‘ button and the project analysis will start. After it completes (a very fast procedure, even for big full Data Abstract for .NET solution), a helper dialog will appear.

Open the NDepend rules explorer (NDepend -> Rule -> View Queries).

As you can see, some entries are marked with a warning sign. These are rules that are potentially violated and were successfully detected by NDepend.

  • The abstract class SomeBaseClass should have a protected constructor instead of a public one.
  • The UtilityClass class should be static.
  • The abstract class SomeBaseClass is potentially dead code (it has assembly-only visibility and isn’t used anywhere it this assembly).
  • The Class1.fUnusedField field is not used anywhere and is thus not needed.

Try NDepend on your project and most likely you’ll find some potentially dangerous code lines.

As you can see, NDepend is mostly language independent, so all tutorials and tips ‘n tricks found at http://www.ndepend.com/ can be used with Oxygene, too.

One last advice: For some rules you won’t be able to navigate to a method or class violating some rule by double-clicking the method’s name.

This happens because in some cases (for navigation purposes) NDepend relies on its built-in language parser (that is limited to C# only) rather than on .PDB information. This custom source code parser is used because .PDB files provide information about source code declarations of concrete methods only, so additional effort is needed to acquire information about source code for namespace and type declarations.

To make it easier to find the problematic method, modify the code validation rule output by adding the full method (or class) name. To do this, just change the “rule output statement” (usually found at the very end of each rule’s query code) from code like this (for the Methods that could have a lower visibility rule):

select new { t, t.Visibility , 
                CouldBeDeclared = t.OptimalVisibility, 
                t.TypesUsingMe }

to this:

select new { t, t.Visibility , 
                CouldBeDeclared = t.OptimalVisibility, 
                t.TypesUsingMe,
                t.FullName }

Now you’ll see the full path (including namespace and class name) of the problematic entry.

P.S.: Don’t be afraid of the NDepend query language. It is a very powerful way to define own or customize existing code validation rules. At the same time, the predefined rules set allows to use the code analyzer without even touching the query language at all (except for the above-mentioned issue).

P.P.S.: Before you say ‘Hey, this thing is way too expensive’, try to estimate how much time it will save you and your team members and how much this time costs.

2 responses to Code Deodorant

  1. “public constructor of an abstract class”
    why shall this hurt(smell)?

    • Usually it is recommended to make abstract class constructors protected. It is not possible to create a new instance of an abstract class, so there is no need to expose its constructor as public.