Making UnitOfWork testable

Question

I'm in the process of refactoring legacy code. Unfortunately it is a WebForms application with very tight coupling and smelly code. Currently access to database is inside models which looks something like this:

var fooModel = new FooModel();
fooModel.Select(sqlConnection); // Accesses db and populates model's data.

I would like to move this logic into Data Access Layer and in the process make it easy to test, so in the future I can migrate to MVC without much of a hassle. Currently the code does not use any ORM, so I have to stick to SqlConnection.

I figured out a way to abstract database access, but I don't know how to make it easy to test. I decided to make this hierarchy:

Controller(currently codebehind) -> Services -> UnitOfWork -> Repositories -> SqlConnection to db.

Basically Controllers can only see Services which use UnitOfWork to access Repositories to make use of transactions and retrieve data.

I created a sample to show you what I mean and what the current problem is. I used simple List instead of actually connecting to database, because it is just a sample to show what I'm looking for and I'm open to changing and rewriting this completely if someone has more knowledge of this kind of problems.

Example service:

public class CustomerService : ICustomerService
{
    public int AddCustomer(Customer customer)
    {
        using (var uof = new UnitOfWork())
        {
            var customerId = uof.CustomerRepository.Add(customer);
            var customerAddressId = uof.CustomerAddressRepository.Add(customer.Address);
            var customerAddress = uof.CustomerAddressRepository.Get(customerAddressId);
            customer.CustomerAddressId = customerAddressId;
            customer.Address = customerAddress;
            uof.CustomerRepository.Update(customer);
            uof.Commit();

            return customerId;
        }
    }

    public void UpdateCustomer(Customer customer)
    {
        using (var uof = new UnitOfWork())
        {
            uof.CustomerRepository.Update(customer);
            uof.Commit();
        }
    }

    public Customer GetCustomerById(int customerId)
    {
        using (var uof = new UnitOfWork())
        {
            return uof.CustomerRepository.Get(customerId);
        }
    }
}

UnitOfWork:

 public class UnitOfWork : IUnitOfWork, IDisposable
 {
    public ICustomerRepository CustomerRepository { get; }
    public ICustomerAddressRepository CustomerAddressRepository { get; }
    private SqlConnection _sqlConnection;
    private SqlTransaction _transaction;

    public UnitOfWork()
    {
        _sqlConnection = new SqlConnection();
        _transaction = _sqlConnection.BeginTransaction();

        CustomerRepository = new CustomerRepository(_sqlConnection);
        CustomerAddressRepository = new CustomerAddressRepository(_sqlConnection);
    }

    public void Commit()
    {
        _transaction.Commit();
    }

    public void Dispose()
    {
        _sqlConnection.Dispose();
    }
}

Example repository:

public class CustomerRepository : ICustomerRepository
{
    private readonly SqlConnection _connection;
    private readonly List<Customer> _customers;

    public CustomerRepository(SqlConnection connection)
    {
        _connection = connection;
        _customers = new List<Customer>();
    }

    public int Add(Customer customer)
    {
        var id = _customers.Count + 1;
        customer.CustomerId = id;
        _customers.Add(customer);
        return id;
    }

    public void Update(Customer newCustomer)
    {
        var index = _customers.FindIndex(f => f.CustomerId == newCustomer.CustomerId);
        _customers.RemoveAt(index);
        _customers.Add(newCustomer);
    }

    public Customer Get(int id)
    {
        return _customers.FirstOrDefault(f => f.CustomerId == id);
    }
}

How controllers(current codebehind) use this:

var customer = _customerService.GetCustomerById(1);
var specificCustomers = _customerService.GetCustomersByIds(new int[] {5, 63, 75});

With the current approach I cannot mock ICustomerRepository in my UnitOfWork class. Also I kind of mixed patterns and what I need together, so I'm not sure if this is the way to go here. What can I do to make repositories in UnitOfWork testable?

Answer 1

This is a common problem in testing. You have class A that uses class B that uses class C. So how do you test class B?

You must be able to mock class C and pass it to class B before you can test B. In your case, we're talking about UnitOfWork depending on ICustomerRepository. The trick is this: if UnitOfWork is responsible for instantiating an instance of ICustomerRepository, you can never hope to mock ICustomerRepository.

You can fix this in two ways:

Via constructor

An instance of ICustomerRepository is passed directly to UnitOfWork. In other words, the relationship between UnitOfWork and ICustomerRepository changes from "UnitOfWork is owner of ICustomerRepository" to "UnitOfWork uses ICustomerRepository". This is a far better place to be, and as a general rule of thumb, if you're using an interface, you should always ask yourself if the class instantiating the implementation should be doing so (more often than not that isn't the case).

Actual creation of ICustomerRepository is done outside UnitOfWork and passed in. When you test, you pass your mock version of ICustomerRepository instead which doesn't connect to the database but acts as you'd expect an instance of ICustomerRepository to behave.

Via property

Your second option is adding a setter to your CustomerRepository property. UnitOfWork still instantiates CustomerRepository in the constructor, but you override with your own after the constructor.

This option is less pleasant because it remains the owner of ICustomerRepository, and yet adding a setter implies UnitOfWork just uses ICustomerRepository. In addition to this, any business logic being performed in your constructor must be executed also on your mock ICustomerRepository instance before being passed, which is of course ugly.

However, it has the advantage that it behaves as it should unless overridden, meaning your code remains untouched except the new setter, and your test class simply adds a mock instance of ICustomerRepository.

Alternatively you could not instantiate ICustomerRepository in UnitOfWork and just leave the setter, but I don't generally like the idea of depending on the setting of properties in order for a class to function properly.

Via an IRepositoryFactory interface

public interface IRepositoryFactory 
{
    ICustomerRepository GetCustomerRepository();
}

public class RepositoryFactory : IRepositoryFactory
{
    private SqlConnection connection;

    public RepositoryFactory(SqlConnection connection) 
    {
        this.connection = connection;
    }

    public ICustomerRepository GetCustomerRepository() 
    {
        return new CustomerRepository(this.connection);
    }
}

public class MockRepositoryFactory : IRepositoryFactory
{
    public MockRepositoryFactory() 
    {
    }

    public ICustomerRepository GetCustomerRepository() 
    {
        return new MockCustomerRepository();
    }
}

I don't know who instantiates UnitOfWork but at this point you would have your instance of IRepository passed to it and it would call getCustomerRepository() in order to obtain the ICustomerRepository to pass to UnitOfWork.

I assume at this point that IRepositoryFactory is dynamically loaded with your application. Once your instance is created, you need not worry about exposing an instance of SqlConnection to the caller of the repository classes. This also gives you the abstraction you require to be able to switch out the implementation of IRepositoryFactory necessary to perform your tests.

The whole point of these abstractions is to be able to change implementation easily. As it stands now, changing implementation is not done easily and therefore testing said implementations is difficult being tightly coupled.