For the FIFA World Championship of 2014, my friends and I wanted a website where we could “bet” on the games. Not for money, just for fun. In the past, we used an Excel file with some fancy formula’s. But then came the idea to write a web application. As the only developer, I accepted the challenge. It was done in a rush, with the technology of the time. Four years later, this gives me an excellent exercise for refactoring. In this series, I will show some techniques I will be using to turn this legacy application into a leaner, more readable, more maintainable and more testable codebase.

The Application

The Application was written quick & dirty in ASP.NET MVC with Angular 1. I started on May 23 2014, and the tournament started on June 12. I continued while games were being played, but the code itself was a mess. Commit messages like “no time for good solution, ui logic in controller” provide proof. If you’re interested, you can still see the state of the code at the end of the 2014 World Championship. In upcoming posts I will dive into more detail of what existed, why it was bad, and how I fixed it.

The bottom line is that it did what it was supposed to do, and it was done (more or less) in time. The fact that it was such an ugly codebase didn’t really matter too much. I was going to fix that all for the European Championship of 2016. But that came faster than expected and apart from adding support for multiple tournaments and bugfixes, nothing was done to improve the quality of the code.

Again, this didn’t really matter as it was an application that would only be used every two years and, well, it worked. But now that I specialize in fixing legacy applications, it should be an interesting experiment to refactor. Let’s take a look at some examples of bad code.

The Comments

Sometimes, I knew there was a problem, and added a comment. This satisfies our conscience when we add the comment, but does nothing to fix the problem. Take this TODO for example:

//TODO: code grew organically with increasing number of ifs, indenting,... Crap

And there are a few other TODO comments in the code, that have never been addressed. I have a theory that most TODO items in code remain there forever, and so these must definitely be addressed when refactoring. I will have to run over the various TODO items and fix them.

Commented or Unused Code

I have previously railed against commenting out code and here I am doing the same. The solution for this is very simple: just remove it.

I take the same approach with unused code. ReSharper does a great job graying out unused code so that you can easily spot it. And it’s trivial to remove it: just move the cursor to the code and press ALT+ENTER. Another tool that does this is NDepend, which we’ll cover later.

Removing commented or unused code is something that you should just do when you see it. It’s no fun going over hundreds and thousands of lines of code to remove code. Just clean up when you encounter it.

Dependency Injection

There is no dependency injection in this project. This can quickly become a pain in a C# application of more than a trivial size. It blocks us from writing fast-running unit tests. For example, it means every Controller instantiates the DbContext by calling the constructor:

public class MatchDetailsController : ApiController
{
    private readonly MatchesContext _contextProvider = new MatchesContext();

    ...
}

Because of this code, we can’t test our controllers without a database. We will have to inject an interface to abstract away the database access.

Angular 1

This was my first real single-page application (SPA), apart from a demo of Durandal. Angular 1 was still all the rage at the time and in fact, it’s called AngularJS and is still being updated. At the time I’m writing this, the latest version is 1.7.2 which was release on June 12, 2018. My application is using 1.5.5, released on April 18, 2016.

I could update to the latest version of AngularJS, but maybe it would make more sense to use Angular (6.x). I haven’t decided yet, because I could also use an entirely different framework. On the other hand, that might take us too far. We’ll get back to this issue later.

What Is Our Goal Exactly?

What is refactoring? No changes in behavior. We’ll probably change and modernize the look and feel and behavior though.

In an enterprise-context, this happens in conjunction with new features. Or at least, it should. Often, teams aren’t given time to refactor, and so they don’t. I’ve written about how we should just do the right thing, regardless of what higher management people tell us. New features are still important, but nobody is stopping you from making small improvements to ugly code that you encounter when implementing these new features.

For this excercise, I’m  not going to add any new features just yet. I’m going to focus purely on refactoring and improving the codebase.

Join me in my next post as I start with an update to Bootstrap.

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.