views:

128

answers:

3

I'm trying to think of a good way to clean up my controllers to make them more testable without having to rely on a constant database connection. I thought I had a decent start by abstracting away my object context with an IObjectContext. This works well for the context, but my next problem is that I have a generic repository that I use in a number of action methods throughout my project (see code below).

In addition to the default constructor, my controller consists of an overload, which accepts an IObjectContext (simple dependency injection). In my unit tests, I can easily mock the IObjectContext. My issue is dealing with my generic repository in various action methods. I could add a number of additional constructor overloads to the controller, but I'm afraid this would get messy, really quickly. Short of doing that, however, I simply haven't been able to think of a clean way to improve testability so that I don't have to rely on a database connection.

Is there a simple solution that I'm overlooking?

/// <summary>
/// Initializes a new instance of the HomeController class
/// </summary>
public HomeController(IObjectContext context)
{
    _context = context;
}

/// <summary>
/// GET: /home/index
/// </summary>
/// <returns>Renders the home page</returns>
public ActionResult Index()
{
    List contacts;
    HomeViewModel model;

    using (IRepository<Contact> repository = new DataRepository<Contact>(_context))
    {
        contacts = new List(repository.GetAll());
    }

    model = new HomeViewModel(contacts);

    return View(model);
}

If I have to go the route of adding additional constructor overloads to accommodate my concerns, I was considering adding a number of private properties (which would deffer instantiation of the repositories until they are needed) to my controllers for each of the repositories that action methods make use of. For example:

private IRepository<Contact> _contactRepository;

private IRepository<Contact> ContactRepository
{
    get
    {
        return _contactRepository ?? (_contactRepository = new DataRepository<Contact>());
    }
}

For unit testing purposes, I'd be able to pre-initialize the repositories using the constructor overloads.

What are your thoughts on this? Am I missing something cleaner that should be obvious?

+1  A: 

Well, I do what your final example shows all the time to inject mocks into my controllers. It does have a little smell to it (designing for testability), but it isn't bad coding and works great for testing.

Will
+3  A: 

First of all, get rid of your current Bastard Injection constructor overloads. With DI, you should only need one constructor, and that's the one that takes all the dependencies. (To enable the ASP.NET MVC run-time to create the Controllers, implement a custom IControllerFactory.)

The next step is to inject all your dependencies through the constructor. When you think it gets messy because there are too many constructor parameters, it's a good sign that you are violating the Single Responsibility Principle. When that happens, you extract an Aggregate Service.

Rinse and repeat :)

Mark Seemann
+1  A: 

Your use of a generic repository is more a dependency-cloaking device than a dependency injection. You should be able to see all of the dependencies a particular Controller uses: a generic repository hides this fact somewhere deep in the entrails of your Controllers which makes maintaining (and unit-testing) the code much more difficult. My suggestion: use concrete repositories.

You could also take a look at domain-driven design stuff.

Igor Brejc