Mr. Escher, Your Software is Ready

We’re now on the ninth installment of Extreme ASP.NET Makeover. In part 8, we discovered that the ScrewTurn Wiki codebase suffers from dependency problems. Tight coupling of implementation classes caused by singletons and static classes result in a morass of objects, which is difficult to refactor. Dependency cycles between objects result in sensitive constructor and method orderings, which can lead to unexpected NullReferenceExceptions after apparently innocuous changes to the code. We refactored AuthorizationChecker to eliminate the dependency problems, but we did not address the elephant in the room—ScrewTurn.Wiki.Host. This article confronts the problems of ScrewTurn.Wiki.Host head-on in an effort to tame its wild ways.

Another Look at Host

Host’s primary purpose is to act as a facade between ScrewTurn Wiki’s plug-ins (aka “Providers”) and ScrewTurn Wiki itself. Host allows plug-ins to do everything from retrieving settings values to sending emails to handling backups to finding users, Wiki pages, namespaces, categories, and more, as shown in Figure 1.

Figure 1 IHostV30

The plug-ins, which implement IProviderV30, are initialized with a Host instance and a configuration string through their Init method:

public interface IProviderV30 {
    void Init(IHostV30 host, string config);
    void Shutdown();
    ComponentInformation Information { get; }
    string ConfigHelpHtml { get; }
}

Looking at Host’s constructor, no dependency problems are immediately evident:

public class Host : IHostV30 {
    public Host() {
        customSpecialTags = new Dictionary<string, CustomToolbarItem>(5);
    }
    ...
}

The plug-in system seems fairly innocuous, but the devil is in the details. Let’s dig a little deeper and see what we find.

The First Step is Realizing You Have a Problem

Given Host’s role as a facade object for the plug-in system and its overly simplistic constructor, I suspect that there is a dependency problem lurking beneath the surface. So I have enrolled Host in Dependency-aholics Anonymous. The first step in this twelve-step program is to make Host’s dependencies obvious, just as we did in part 8 when working with AuthorizationChecker.

I’m not too concerned at the moment about static helper classes, such as ScrewTurn.Wiki.Tools or ScrewTurn.Wiki.Formatter. Static helper classes are often a rich source of inspiration for domain concepts. For example, if you have helper methods for performing arithmetic operations on currency, introduce a Money class to encapsulate the data and behavior. If you have helper methods comparing whole dates, introduce a Date class. Yes, .NET has a DateTime class, but it includes both the date and the time. Don’t be afraid to create a Date or Time class that is tailored to the needs of your application. If you deal with financial data, working with a strongly-typed FiscalPeriod class is going to be a lot easier than passing around DateTime objects and remembering to call Helpers.ConvertToFiscal(someDate) to ensure that the DateTime is properly aligned to a fiscal period.

But static helpers aren’t our concern at the moment because these are most often simple stateless methods that have little to no coupling to the rest of the application. We are looking for hidden dependencies that wind their tentacles insidiously throughout the application. The dependencies that have dependencies that have dependencies ad infinitum, resulting in unnecessary coupling and rigidity in the codebase. To find these, we need look no further than the Singletons to which Host delegates much of its responsibilities.

As we saw, the Host class delegates to a whole host of Singletons (bad pun intended), including Settings, Users, Pages, Snippets, NavigationPaths, and AuthWriter. Each of these Singletons refers to other Singletons, creating a morass of dependencies. Before we start refactoring Host, we need some tests in place to ensure that we are not breaking anything. Configuration of the application and infrastructure is done by the ApplicationBootstrapper, which we introduced in Part 8. Our tests are going to execute ApplicationBootstrapper.Configure, as shown in Figure 2, and then verify that IHostV30 can be resolved from the IoC container as well as that various Singletons are still configured properly. Note that the IoC is initialized to null after every test to ensure that the tests don’t interfere with one another.

Figure 2 Tests Execute ApplicationBootstrapper.Configure

[TestFixture]
public class AfterTheApplicationBootstraperIsExecuted {
    [Test]
    public void IHostCanBeResolvedFromTheContainer() {
        IoC.Resolve<IHostV30>();
    }
 
    [Test]
    public void CacheInstanceIsInitialized() {
        Assert.That(Cache.Instance, Is.Not.Null);
    }
 
    [Test]
    public void SettingsInstanceIsInitialized() {
        Assert.That(Settings.Instance, Is.Not.Null);
    }
 
    [Test]
    public void AuthReaderInstanceIsInitialized() {
        Assert.That(AuthReader.Instance, Is.Not.Null);
    }
 
    [Test]
    public void AuthWriterInstanceIsInitialized() {
        Assert.That(AuthWriter.Instance, Is.Not.Null);
    }
 
    [SetUp]
    public void SetUp() {
        new ApplicationBootstrapper().Configure();
    }
 
    [TearDown]
    public void TearDown() {
        IoC.Initialize(null);
    }
}

With these tests in place, let’s start refactoring Host to expose its dependency problems.

Going Around in Circles

We successfully refactored away Host’s dependency on Settings.Instance by allowing the IoC container to pass Host an instance of ISettings via its constructor. It seems like a simple matter to continue refactoring away Host’s dependencies on the other Singletons, but trouble is brewing just around the corner. The next Singleton we extract is Users.Instance. Following exactly the same procedure as Settings.Instance, we discover a horrible secret, as shown in Figure 3.

Figure 3 CircularDependencyException

Let’s take a look at the constructors for Host and Users:

public Host(ISettings settings, IUsers users) {
    customSpecialTags = new Dictionary<string, CustomToolbarItem>(5);
    this.settings = settings;
    this.users = users;
}

public Users(IHostV30 host) {
    this.host = host;
}

So Host depends on Users and Users depends on Host. Before the refactoring, this dependency cycle wasn’t obvious. The two components are in fact much more tightly coupled to one another than expected. Let’s take a deeper look at Users to find out where this coupling is coming from.

The only reason for Users to reference Host is to raise events on behalf of the Users class. We can quickly and easily remove this dependency by allowing Users to raise its own events, as shown in Figure 4.

Figure 4 Users Raises Its Own Events

public class Users : IUsers {
  public event EventHandler<UserAccountActivityEventArgs> UserAccountChanged;
 
  private void OnUserAccountChanged(UserInfo user,
                                    UserAccountActivity activity) {
    if(UserAccountChanged != null) {
      UserAccountChanged(this, 
                         new UserAccountActivityEventArgs(user, activity));
    }
  }

  public event EventHandler<UserGroupActivityEventArgs> UserGroupChanged;
 
  private void OnUserGroupChanged(UserGroup group,
                                  UserGroupActivity activity) {
    if(UserGroupChanged != null) {
      UserGroupChanged(this,
                       new UserGroupActivityEventArgs(group, activity));
    }
  }
}

Host can now subscribe to the Users.UserAccountChanged and Users.UserGroupChanged events and re-broadcast those events to interested listeners without breaking the facade by exposing the Users class directly, as Figure 5 shows.

Figure 5 Host Can Subscribe to the Users.UserAccountChanged and Users.UserGroupChanged Events

public class Host : IHostV30 {
    public Host(ISettings settings, IUsers users) {
        customSpecialTags = new Dictionary<string, CustomToolbarItem>(5);
        this.settings = settings;
        this.users = users;
        users.UserAccountChanged += OnUserAccountActivity;
        users.UserGroupChanged += OnUserGroupActivity;
    }

    public event
           EventHandler<UserAccountActivityEventArgs> UserAccountActivity;
 
    private void OnUserAccountActivity(object sender,
                                       UserAccountActivityEventArgs args) {
        if(UserAccountActivity != null) {
            UserAccountActivity(this, args);
        }
    }
 
    public event 
           EventHandler<UserGroupActivityEventArgs> UserGroupActivity;
 
    private void OnUserGroupActivity(object sender, 
                                     UserGroupActivityEventArgs args) {
        if(UserGroupActivity != null) {
            UserGroupActivity(this, args);
        }
    }
}

Users no longer needs a reference to Host and it can be safely removed from Users’ constructor. We have broken the circular dependency between Host and Users. Running our tests, we see that everything is green again, as shown in Figure 6.

Figure 6 Unit Test Success

Before we move on, notice how the circular dependency problem wasn’t immediately noticeable when the classes were linked together using Singletons. Singletons, due to their global nature, make it easy for dependencies to spill between classes. Using the dependency injection principle, it is much easier to see a class’s dependencies because most of them are in the constructor and the remainder are method parameters.

Don’t You Forget about IoC

With apologies to Billy Idol, we cannot forget about the container. It is responsible for creating and wiring together our dependencies. Looking at ScrewTurnWikiInitializationTask, we see the following:

Users.Instance = new Users();

Classes that take a dependency on IUsers are going to get an instance of Users supplied through their constructor, but classes that access Users.Instance are going to get a different Users instance. We can solve this minor problem by allowing the IoC container to supply ScrewTurnWikiInitializationTask with an IUsers instance through its constructor. We now have:

public ScrewTurnWikiInitializationTask(IHostV30 host, 
         ISettingsStorageProviderV30 settingsStorageProvider, IUsers users) {
    this.host = host;
    this.settingsStorageProvider = settingsStorageProvider;
    this.users = users;
}

public void Execute() {
    ...
    Users.Instance = users;
    ...
}

While we’re talking about the container, let’s look at HostAnProviderRegistration:

public class HostAndProviderRegistration : IContainerStartupTask {
    public void Execute(IWindsorContainer container) {
        container.Register(
            Component.For<IHostV30>().ImplementedBy<Host>(),
            Component.For<ISettings>().ImplementedBy<Settings>(),
            Component.For<IUsers>().ImplementedBy<Users>());
 
        ...
    }
}

You’ll notice that we’re starting to get a lot of repetitive code. This is a great place to establish a convention rather than manually configuring each dependency. Let’s create a ScrewTurn.Wiki.Hosting namespace and register each dependency in that namespace against its first interface:

public class HostAndProviderRegistration : IContainerStartupTask {
    public void Execute(IWindsorContainer container) {
        container.Register(
            AllTypes.FromAssembly(Assembly.GetExecutingAssembly())
                .Where(x => x.Namespace.StartsWith("ScrewTurn.Wiki.Hosting"))
                .WithService.FirstInterface()
        );
 
        ...
    }
}

As we refactor dependencies, we need only place them in this namespace (achieved by moving them to the Hosting folder in ScrewTurn.Wiki.Core) and they will automatically be registered correctly in the IoC container.

The Journey Continues

Users depends on the Settings.Instance and Pages.Instance Singletons. These are quickly extracted into constructor dependencies as before. Pages gets the same treatment as Users—moving to ScrewTurn.Wiki.Hosting namespace (for automatic IoC registration through our convention), interface extraction, and pushing dependent Singletons to constructor parameters. (Additional tests were added to ensure that IPages can be resolved from the IoC container and that Pages.Instance is properly initialized.)

Pages has the same problem as Users in that it uses Host to raise events on its behalf. The same procedure is applied whereby Pages is refactored to raise its own events, which Host then registers for and re-raises to interested listeners. Once again, we’ve broken an unnecessary coupling between two classes, Pages and Host this time. For those interested in the details, you can check out the code download associated with the article.

Pages makes use of many other Singletons and the refactoring continues in much the same vein until we get to Users. Once we move Users into a constructor parameter, our tests break. Pages depends on Users and Users depends on Pages. We’ve uncovered another circular dependency issue. Let’s take a closer look.

The circular dependency problem between Pages and Users is due to each class implementing part of a larger notification feature. Once again this was not obvious when the two classes were coupled together via Singleton instances. Although the Single Responsibility Principle (SRP) would encourage us to have a separate Notification class, our primary job right now is to disentangle Pages and Users. Once the responsibility for notifications has been moved to one class or the other, it can be separated more easily into a separate Notification class. Let’s work on moving it to Pages.

We’ll start by adding notification-related methods on IUsers to IPages (we won’t remove the methods from IUsers or Users yet):

public interface IPages {
    ...
    bool SetEmailNotification(UserInfo user, PageInfo page,
                              bool pageChanges, bool discussionMessages);
    bool SetEmailNotification(UserInfo user, NamespaceInfo nspace, 
                              bool pageChanges, bool discussionMessages);
    void GetEmailNotification(UserInfo user, PageInfo page, 
                      out bool pageChanges, out bool discussionMessages);
    void GetEmailNotification(UserInfo user, NamespaceInfo nspace, 
                      out bool pageChanges, out bool discussionMessages);
    UserInfo[] GetUsersToNotifyForPageChange(PageInfo page);
    UserInfo[] GetUsersToNotifyForDiscussionMessages(PageInfo page);
}

We will implement these methods on Pages by temporarily delegating to Users:

public bool SetEmailNotification(UserInfo user, PageInfo page,
                                 bool pageChanges, bool discussionMessages) {
    return users.SetEmailNotification(user, page, 
                                      pageChanges, discussionMessages);
}

The other methods are implemented similarly. We can now look for usages of those methods on IUsers and change them to reference the same methods on IPages instead. The only usages for the IUsers methods should now be by Pages and the IUsers methods can be removed. You’ll have to temporarily cast the IUsers instances in the delegating methods to Users as the methods no longer exist on the IUsers interface:

public bool SetEmailNotification(UserInfo user, PageInfo page,
                                 bool pageChanges, bool discussionMessages) {
    return ((Users)users).SetEmailNotification(user, page,
                                      pageChanges, discussionMessages);
}

Time to pack up the notification-related methods on Users and move them to their new home on Pages.

And That’s a Wrap

Simply by making the dependencies between classes explicit via dependency injection, we revealed a number of circular dependencies lurking in the ScrewTurn Wiki codebase. Circular dependencies between components effectively turn the individual components into one large super-structure, which is difficult to modify. Changes ripple through these super-structures and resulting in unexpected breakages after seemingly innocuous modifications.

Applying the same techniques to Host’s other dependencies, we remove Host’s use of singletons altogether. (For the curious reader, the code is available for download from MSDN Code Gallery for Extreme ASP.NET Makeover.) Host’s constructor signature now looks like this:

public Host(ISettings settings, IAuthWriter authWriter, IUsers users,
         IPages pages, ISnippets snippets, INavigationPaths navigationPaths)

Its dependencies are obvious in a way that they were not before. Host has a fair number of dependencies, but it is acting as a facade between ScrewTurn Wiki and the plug-ins. So it is not overly worrying.

We can start asking ourselves meta questions about our software:

·        Do these dependencies make sense?

·        Should some of these dependencies be combined/split?

·        How do the dependencies relate to one another and can those relationships be improved?

We can reason about the overall structure of our software and improve it because that structure is more apparent. In the end, our software becomes more flexible, more testable, and more resilient in the face of change.

Acknowledgements

I would like to thank Dario Solera, creator of ScrewTurn Wiki, for being brave and open in allowing me to use ScrewTurn Wiki as the brownfield application throughout this series. He has already taken the constructive criticism provided in earlier articles and used it to improve the latest builds of ScrewTurn Wiki. Kudos to Dario for permitting all of us to learn together out in the open.

Thank you to my two occasional co-authors, Kyle Baley and Donald Belcham, for their assistance on parts 6 and 7. Your insights were invaluable in pushing the series forward. Dario has agreed to allow ScrewTurn Wiki to be used as the sample brownfield application for Kyle and Donald’s upcoming book, Brownfield Application Development in .NET, published by Manning. If you want more information about improving brownfield applications, their book is an excellent resource.

A big thanks to Howard Dierking for giving me the latitude and encouragement to take the series where I thought would be most useful and practical. Thanks also to Gary Clarke and the rest of the MSDN Magazine staff for their helpful suggestions, patience, and quick turn-around while editing the series.

Finally, I would like to offer special thanks to my spouse, Risa Kawchuk, and my two sons, Daegan and Gareth, for their understanding in not having as much spouse/daddy time in the last few months as I’ve toiled away evenings and weekends on these articles and screencasts.