Using the builder pattern to build mock objects and nested builders

Question

No, this question has no roosting construction workers.

I'm not sure if what I'm doing is a good use of the builder pattern or whether its a bastardization of what its intended purpose was. I'm trying to set up mocks in my tests and find myself repeating myself often to set up the mocks, so I've tried to simplify it using a builder to make my mocks for me. I'm not sure if what I have here is a good pattern for this or whether there's a better one.

I have set up a builder to make my API controller for me with mock objects parsed in. I'm using those builder methods to call methods on contained builders that it is using to make the relevant mocks for the controller its building. So there are nested builders within this controller builder that I'm just setting up through the controller's builder methods. Here is my outer ControllerBuilder:

public class DoStuffControllerBuilder
{
    private Mock<IRepository> _mockRepo;
    private MockStuffLoggerBuilder _loggerBuilder;
    private MockAzureQueueManagerBuilder _queueManagerBuilder;

    private DoStuffControllerBuilder()
    {
        _mockRepo = new Mock<IRepository>();
        _loggerBuilder = MockStuffLoggerBuilder.GetBuilder();
        _queueManagerBuilder = MockAzureQueueManagerBuilder.PrepareBuilder();
    }

    public bool WasEnqueueCalledOnQueueManager { get { return _queueManagerBuilder.WasSomethingQueued; } }
    public Message LastQueuedMessage { get { return _queueManagerBuilder.LastQueuedMessage; } }
    public bool CreateJobLogWasCalled { get { return _loggerBuilder.CreateJobLogWasCalled; } }
    public int CreateJobLogID { get { return _loggerBuilder.CreateJobLogID; } }

    public static DoStuffControllerBuilder GetBuilder()
    {
        return new DoStuffControllerBuilder();
    }

    public DoStuffController Build()
    {
        return new DoStuffController(_mockRepo.Object, _loggerBuilder.BuildMock(), _queueManagerBuilder.BuildMock());
    }

    public DoStuffControllerBuilder WithRememberEnqueueWasCalled()
    {
        _queueManagerBuilder.WithRememberSomethingWasQueued();
        return this;
    }

    public DoStuffControllerBuilder WithRememberLastQueuedMessage()
    {
        _queueManagerBuilder.WithRememberLastQueuedMessage();
        return this;
    }

    public DoStuffControllerBuilder WithRememberCreateLogWasCalled()
    {
        _loggerBuilder.WithRememberCreateLogWasCalled();
        return this;
    }

    public DoStuffControllerBuilder WithRememberCreateLogID()
    {
        _loggerBuilder.WithRememberCreateLogID();
        return this;
    }
}

The code for my inner Azure queue builder is below. Here, I'm using it to set up a mock object to verify something was called later on and remember values. More functionality may be added later.

public class MockAzureQueueManagerBuilder
{
    private Mock<IAzureQueueManager> _mockManager;
    private bool _rememberWasQueued, _rememberLastMessage;
    private Message _lastQueuedMessage;
    private bool _wasSomethingQueued;

    private MockAzureQueueManagerBuilder()
    {
        _mockManager = new Mock<IAzureQueueManager>();
    }

    public bool WasSomethingQueued { get { return _wasSomethingQueued; } }
    public Message LastQueuedMessage { get { return _lastQueuedMessage; } }

    public static MockAzureQueueManagerBuilder PrepareBuilder()
    {
        return new MockAzureQueueManagerBuilder();
    }

    public IAzureQueueManager BuildMock()
    {
        var setup = _mockManager.Setup(q => q.EnqueueMessageByJobs(It.IsAny<Message>()));
        if (_rememberWasQueued || _rememberLastMessage)
            setup.Callback<Message>(message =>
            {
                if (_rememberLastMessage)
                    _lastQueuedMessage = message;
                if (_rememberWasQueued)
                    _wasSomethingQueued = true;
            });
        return _mockManager.Object;
    }

    public MockAzureQueueManagerBuilder WithRememberSomethingWasQueued()
    {
        _rememberWasQueued = true;
        return this;
    }

    public MockAzureQueueManagerBuilder WithRememberLastQueuedMessage()
    {
        _rememberLastMessage = true;
        return this;
    }    
}

Finally, here it is in action:

public void AddStuffShouldEnqueueAMessageWithJobs()
{
    // given an ID of 1
    var builder = DoStuffControllerBuilder.GetBuilder();
    var controller = builder.WithRememberEnqueueWasCalled().WithRememberLastQueuedMessage().Build();
    var options = new Options {Entities = new List<int> {1}};

    // when we post that entity to the controller
    controller.Post(options);

    // then it calls EnqueueMessageByJobs
    Assert.IsTrue(builder.WasEnqueueCalledOnQueueManager, "No message was queued");
    // and the ID is the given ID
    var lastMessage = builder.LastQueuedMessage;
    Assert.AreEqual(1, lastMessage.ID, "The ID on the message is not what was expected");
    // and it has at least 1 job
    Assert.IsTrue(lastMessage.JobsToDo.Any(), "The message has been queued with no jobs to do");
}

Any tips to get me building nicer much appreciated.

Answer 1

Factory Method vs Constructor

In your builder classes you have opted to use factory methods instead of constructors. While this is not necessarily wrong, exposing a factory method generally implies a semantically different operation than constructor invocation. From the Constructor Design guidelines:

Consider using a static factory method instead of a constructor if the semantics of the desired operation do not map directly to the construction of a new instance, or if following the constructor design guidelines feels unnatural.

In this case, I don't think exposing factory methods on your builders is necessary, especially considering that all they do internally is call their respective instance constructors.

Naming Conventions

Generally speaking, methods should start with verbs. This is especially the case when implementing the builder pattern. As a result, I would remove the With prefix from the appropriate methods (e.g. WithRememberEnqueueWasCalled() becomes RememberEnqueueWasCalled()). This also makes the code more readable in my opinion.

Both MockAzureQueueManagerBuilder and MockStuffLoggerBuilder have BuildMock methods. I think the Mock post-fix is redundant here since building a mock is already implied by the name of the class.

Unit Testing

When unit-testing, the general rule-of-thumb is to only test a single concern. In the unit test you have provided, you seem to be testing three separate concerns since you're asserting against three values on three different objects. I would split the unit test in question into three separate ones.

Result

DoStuffControllerBuilder

public class DoStuffControllerBuilder
{
    // these fields can be readonly
    private readonly Mock<IRepository> _mockRepo;
    private readonly MockStuffLoggerBuilder _loggerBuilder;
    private readonly MockAzureQueueManagerBuilder _queueManagerBuilder;

    public DoStuffControllerBuilder()
    {
        _mockRepo = new Mock<IRepository>();
        _loggerBuilder = new MockStuffLoggerBuilder();
        _queueManagerBuilder = new MockAzureQueueManagerBuilder();
    }

    public bool WasEnqueueCalledOnQueueManager
    {
        get { return _queueManagerBuilder.WasSomethingQueued; }
    }

    public Message LastQueuedMessage
    {
        get { return _queueManagerBuilder.LastQueuedMessage; }
    }

    public bool CreateJobLogWasCalled
    {
        get { return _loggerBuilder.CreateJobLogWasCalled; }
    }

    public int CreateJobLogID
    {
        get { return _loggerBuilder.CreateJobLogID; }
    }

    public DoStuffController Build()
    {
        return new DoStuffController(_mockRepo.Object, _loggerBuilder.Build(), _queueManagerBuilder.Build());
    }

    public DoStuffControllerBuilder RememberEnqueueWasCalled()
    {
        _queueManagerBuilder.RememberSomethingWasQueued();
        return this;
    }

    public DoStuffControllerBuilder RememberLastQueuedMessage()
    {
        _queueManagerBuilder.RememberLastQueuedMessage();
        return this;
    }

    public DoStuffControllerBuilder RememberCreateLogWasCalled()
    {
        _loggerBuilder.RememberCreateLogWasCalled();
        return this;
    }

    public DoStuffControllerBuilder RememberCreateLogID()
    {
        _loggerBuilder.RememberCreateLogID();
        return this;
    }
}

MockAzureQueueManagerBuilder

public class MockAzureQueueManagerBuilder
{
    private readonly Mock<IAzureQueueManager> _mockManager;
    private bool _rememberWasQueued, _rememberLastMessage;
    private Message _lastQueuedMessage;
    private bool _wasSomethingQueued;

    public MockAzureQueueManagerBuilder()
    {
        _mockManager = new Mock<IAzureQueueManager>();
    }

    public bool WasSomethingQueued
    {
        get { return _wasSomethingQueued; }
    }

    public Message LastQueuedMessage
    {
        get { return _lastQueuedMessage; }
    }

    public IAzureQueueManager Build()
    {
        var setup = _mockManager.Setup(q => q.EnqueueMessageByJobs(It.IsAny<Message>()));
        if (_rememberWasQueued || _rememberLastMessage)
            setup.Callback<Message>(message =>
            {
                if (_rememberLastMessage)
                    _lastQueuedMessage = message;
                if (_rememberWasQueued)
                    _wasSomethingQueued = true;
            });
        return _mockManager.Object;
    }

    public MockAzureQueueManagerBuilder RememberSomethingWasQueued()
    {
        _rememberWasQueued = true;
        return this;
    }

    public MockAzureQueueManagerBuilder RememberLastQueuedMessage()
    {
        _rememberLastMessage = true;
        return this;
    }
}

Unit Tests

public void ShouldEnqueueMessage()
{
    var builder = new DoStuffControllerBuilder()
        .RememberEnqueueWasCalled()
        .Build();

    var options = new Options
    {
        Entities = new List<int> { 1 }
    };

    // when we post that entity to the controller
    controller.Post(options);

    // then it calls EnqueueMessageByJobs
    Assert.IsTrue(builder.WasEnqueueCalledOnQueueManager, "No message was queued");
}

public void EnqueuedMessageShouldBeLast()
{
    var builder = new DoStuffControllerBuilder()
        .RememberLastQueuedMessage()
        .Build();

    var options = new Options
    {
        Entities = new List<int> { 1 }
    };

    // when we post that entity to the controller
    controller.Post(options);

    // the ID is the given ID
    var lastMessage = builder.LastQueuedMessage;
    Assert.AreEqual(1, lastMessage.ID, "The ID on the message is not what was expected");
}

public void EnqueuedMessageShouldHaveJobs()
{
    // given an ID of 1
    var builder = new DoStuffControllerBuilder()
        .RememberLastQueuedMessage()
        .Build();

    var options = new Options
    {
        Entities = new List<int> { 1 }
    };

    // when we post that entity to the controller
    controller.Post(options);

    // it has at least 1 job
    Assert.IsTrue(builder.LastQueuedMessage.JobsToDo.Any(), "The message has been queued with no jobs to do");
}

Answer 2

As a rule, you should prefer to inject dependencies into your objects; this gives you a lot more flexibility going forwards.

Example:

// these fields can be readonly
private readonly Mock<IRepository> _mockRepo;
private readonly MockStuffLoggerBuilder _loggerBuilder;
private readonly MockAzureQueueManagerBuilder _queueManagerBuilder;

public DoStuffControllerBuilder(Mock<IRepository> mockRepo, MockStuffLoggerBuilder loggerBuilder, MockAzureQueueManagerBuilder queueManagerBuilder)
{
    this._mockRepo = mock;
    this._loggerBuilder = loggerBuilder;
    this._queueManagerBuilder = queueManagerBuilder;
}

public static DoStuffControllerBuilder create() {
    return new DoStuffControllerBuilder
        (new Mock<IRepository>()
        ,new MockStuffLoggerBuilder()
        ,new MockAzureQueueManagerBuilder()
        );
}

Note that you don't normally want your production code to be aware of mock objects. So it's a bit more likely that you should be doing something like:

// these fields can be readonly
private readonly IRepository _mockRepo;
private readonly StuffLoggerBuilder _loggerBuilder;
private readonly AzureQueueManagerBuilder _queueManagerBuilder;

public DoStuffControllerBuilder(IRepository mockRepo, StuffLoggerBuilder loggerBuilder, AzureQueueManagerBuilder queueManagerBuilder)
{
    this._mockRepo = mock;
    this._loggerBuilder = loggerBuilder;
    this._queueManagerBuilder = queueManagerBuilder;
}

public static DoStuffControllerBuilder create() {
    return new DoStuffControllerBuilder
        (new Mock<IRepository>()
        ,new MockStuffLoggerBuilder()
        ,new MockAzureQueueManagerBuilder()
        );
}

If you intend that others can extend your builder with their own implementation, then return this; is a bad idea. Look for information on builder patterns with fluent interfaces to get a feel for the details.