In my series of fixing a real-world legacy application, I’ve already improved the code in some big blocks:

But fixing legacy applications often means making many smaller improvements. Many of these are often a matter of personal opinion. And when multiple developers do agree on an issue, they might not agree on a particular solution.

The best way to avoid these nonconstructive discussions, is to have a tool to automate such checks. For .NET, NDepend is more or less the de facto standard and is an extremely useful tool in legacy applications.

Installing

Installing NDepend is a little different. You just download it from the site and unzip it to a location of your choice. It’s best not to put it in the “Program Files” folder, because this will lead to issues with Windows. I put it in my user’s AppData/Local folder.

Update: Patrick from NDepend reached out to me and pointed out that there is also an Azure DevOps (formerly Visual Studio Online, formerly TFS) extension. You can find it here.

Using NDepend

There are several ways of using NDepend:

  • The standalone Visual NDepend: use this if you don’t want to tie it in with Visual Studio
  • Visual Studio integration: run the Visual Studio extension installer from the folder you unzipped NDepend to
  • Console: use this to run NDepend from the console; this is useful for integration with other tools

You can also add NDepend to your CI build, but that is currently out of scope for my application. Though that could come up later.

Run an Analysis

I’m going to use the Visual Studio integration for now. In Visual Studio, I open my solution, and go to the NDepend menu to choose “Attach New NDepend Project to Current VS Solution.” After choosing the assemblies to include in the analysis (only one in my case), I can run the analysis and get a report:

My general score of technical debt is a C. Not too shabby, but there’s definitely room for improvement. I’m sticking with the default rules for now. When I encounter any rules I don’t agree with, I can edit or remove them.

Set a Baseline

One thing I still need to do is to choose a baseline. This is where all future results will be compared against. This is useful to know how you’re evolving. Click on the “define” link to choose your baseline. This should allow us to track our progress.

Fix Issues

Now let’s look at some of our most pressing issues. I seem to be violating 3 critical rules. If I click on the number, I can see the details:

If I double-click on any of these three, I can see which pieces of code are violating the rule:

The NDepend dashboard should be your starting point for step-by-step refactorings. They point to issues in your code. There’s no need to fix them all at once, but they provide a great insight into where your priorities should be when fixing issues.

Write Tests Before Fixing

Fixing the issues mentioned in NDepend will require some refactoring. But how can I know I don’t break anything while refactoring? I will need to have a decent test suite first.

Unfortunately, the application is missing a lot of tests. Many codepaths were tested manually. Take a look at this method for example:

[HttpGet]
public IList<Points> Get(int id)
{
    var result = new List<Points>();
    var bettings = _context.Bettings.Where(x => x.Match.TournamentId == id).ToList();
    var matches = _context.Matches.Where(x => x.TournamentId == id).ToList();
    var users = new ApplicationDbContext().Users.ToList();
    var usersWithCorrectTopScorer = _context.TopScorers.Where(x => x.TournamentId == id && x.IsCorrect).Select(x => x.UserName).ToList();

    foreach (var user in users)
    {
        var points = new Points { UserName = user.UserName };

        if (usersWithCorrectTopScorer.Contains(user.UserName))
        {
            points.Total += 2;
        }

        result.Add(points);
    }

    foreach (var match in matches)
    {
        if (!match.HomeScore.HasValue && !match.AwayScore.HasValue)
        {
            continue;
        }

        var bettingsForMatch = bettings.Where(x => x.MatchId == match.Id);
        foreach (var bettingForMatch in bettingsForMatch)
        {
            var userName = bettingForMatch.UserName;
            var points = result.SingleOrDefault(x => x.UserName == userName);
            if (points == null)
            {
                points = new Points { UserName = userName };
                result.Add(points);
            }

            if (bettingForMatch.HomeScore == match.HomeScore && bettingForMatch.AwayScore == match.AwayScore)
            {
                points.Total += 2;
            }
            else if (BettingHasCorrectResult(bettingForMatch, match))
            {
                points.Total += 1;
            }
        }
    }

    return result;
}

If I refactor this intensively, how can I know that I haven’t broken anything? The best solution here is to write several tests first. You can’t be sure that your tests will cover everything, but it’s a better safety net than a few manual tests.

Here too, NDepend can help. It’s out of scope for this article, but NDepend can take code coverage into account. It’s even possible to define rules based on code coverage.

Having tests also provides a better development experience. Instead of having to start the application up to test your changes, you can just run the tests.

Thanks to the dependency injection we introduced earlier, we can now easily write tests and mock out the database. Then we will feel more confident when refactoring. This is what I’ll be doing for the coming weeks, and then we’ll come back for another look at our NDepend score.

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes:

<a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <s> <strike> <strong>

This site uses Akismet to reduce spam. Learn how your comment data is processed.