How could I refactor this factory-type method and database call to be testable?

Question

I'm trying to learn how to do Unit Testing and Mocking. I understand some of the principles of TDD and basic testing. However, I'm looking at refactoring the below code that was written without tests and am trying to understand how it needs to change in order to make it testable.

public class AgentRepository
{

public Agent Select(int agentId)
{
    Agent tmp = null;
    using (IDataReader agentInformation = GetAgentFromDatabase(agentId))
    {
        if (agentInformation.Read())
        {
            tmp = new Agent();
            tmp.AgentId = int.Parse(agentInformation["AgentId"].ToString());
            tmp.FirstName = agentInformation["FirstName"].ToString();
            tmp.LastName = agentInformation["LastName"].ToString();
            tmp.Address1 = agentInformation["Address1"].ToString();
            tmp.Address2 = agentInformation["Address2"].ToString();
            tmp.City = agentInformation["City"].ToString();
            tmp.State = agentInformation["State"].ToString();
            tmp.PostalCode = agentInformation["PostalCode"].ToString();
            tmp.PhoneNumber = agentInformation["PhoneNumber"].ToString();
        }
    }

    return tmp;
}

private IDataReader GetAgentFromDatabase(int agentId)
{
    SqlCommand cmd = new SqlCommand("SelectAgentById");
    cmd.CommandType = CommandType.StoredProcedure;

    SqlDatabase sqlDb = new SqlDatabase("MyConnectionString");
    sqlDb.AddInParameter(cmd, "AgentId", DbType.Int32, agentId);
    return sqlDb.ExecuteReader(cmd);
}

}

These two methods are in a single class. The database-related code in the GetAgentFromDatabase is related to Enterprise Libraries.

How would I be able to go about making this testable? Should I abstract out the GetAgentFromDatabase method into a different class? Should GetAgentFromDatabase return something other than an IDataReader? Any suggestions or pointers to external links would be greatly appreciated.

Answer 1

You're correct about moving GetAgentFromDatabase() into a separate class. Here's how I redefined AgentRepository:

public class AgentRepository {
    private IAgentDataProvider m_provider;

    public AgentRepository( IAgentDataProvider provider ) {
        m_provider = provider;
    }

    public Agent GetAgent( int agentId ) {
        Agent agent = null;
        using( IDataReader agentDataReader = m_provider.GetAgent( agentId ) ) {
            if( agentDataReader.Read() ) {
                agent = new Agent();
                // set agent properties later
            }
        }
        return agent;
    }
}

where I defined the IAgentDataProvider interface as follows:

public interface IAgentDataProvider {
    IDataReader GetAgent( int agentId );
}

So, AgentRepository is the class under test. We'll mock IAgentDataProvider and inject the dependency. (I did it with Moq, but you can easily redo it with a different isolation framework).

[TestFixture]
public class AgentRepositoryTest {
    private AgentRepository m_repo;
    private Mock<IAgentDataProvider> m_mockProvider;

    [SetUp]
    public void CaseSetup() {
        m_mockProvider = new Mock<IAgentDataProvider>();
        m_repo = new AgentRepository( m_mockProvider.Object );
    }

    [TearDown]
    public void CaseTeardown() {
        m_mockProvider.Verify();
    }

    [Test]
    public void AgentFactory_OnEmptyDataReader_ShouldReturnNull() {
        m_mockProvider
            .Setup( p => p.GetAgent( It.IsAny<int>() ) )
            .Returns<int>( id => GetEmptyAgentDataReader() );
        Agent agent = m_repo.GetAgent( 1 );
        Assert.IsNull( agent );
    }

    [Test]
    public void AgentFactory_OnNonemptyDataReader_ShouldReturnAgent_WithFieldsPopulated() {
        m_mockProvider
            .Setup( p => p.GetAgent( It.IsAny<int>() ) )
            .Returns<int>( id => GetSampleNonEmptyAgentDataReader() );
        Agent agent = m_repo.GetAgent( 1 );
        Assert.IsNotNull( agent );
                    // verify more agent properties later
    }

    private IDataReader GetEmptyAgentDataReader() {
        return new FakeAgentDataReader() { ... };
    }

    private IDataReader GetSampleNonEmptyAgentDataReader() {
        return new FakeAgentDataReader() { ... };
    }
}

(I left out the implementation of class FakeAgentDataReader, which implements IDataReader and is trivial -- you only need to implement Read() and Dispose() to make the tests work.)

The purpose of AgentRepository here is to take IDataReader objects and turn them into properly formed Agent objects. You can expand the above test fixture to test more interesting cases.

After unit-testing AgentRepository in isolation from the actual database, you will need unit tests for a concrete implementation of IAgentDataProvider, but that's a topic for a separate question. HTH