The State Pattern: 10 Years of .Net Compressed into Weeks #8

This post is part of a blog series ASP.Net 10 Years On. Even though this is part of a series I have tried to make each post standalone.

In this post we look to finish off the competition logic. We have one part of the logic left to implement which is to prevent the pick winner logic being selected twice if the user were to refresh their browser window.

The logic is as follows: once a competition has been closed and a winner selected, flag it and stop any subsequent attempts to select a winner. Our requirements also state that the competition must be protected from being closed before the official closing date.

We could extend our PickWinner method so that it looks as follows:

public void PickWinner()
{
    if (this.CompetitionStatistics.Competition.ClosingDate < DateTime.Now)
    {
        // the competition closing date has not yet passed
        return;
    }

    if (this.CompetitionStatistics.Competition.IsClosed)
    {
        // the competition has been closed.
        return;
    }

    if (CompetitionStatistics.HasCorrectAnswers)
    {
        CompetitionStatistics.Competition.Winner = GetCompetitionWinner();
        DomainEvents.Raise(new WinnerSelectedEvent(CompetitionStatistics.Competition));
    }
}

This looks pretty straight forward but, I'm of the opinion that:

Every IF statement you write is a potential bug.

Granted these simple little IF statements aren't too offensive, but over time more will creep in and my code base will be littered with more chances for errors in the future.

If we break down this logic we have three separate behaviours when PickWinner is called - one when the competition is active and cannot be closed, the second when the competition is no longer active but a winner has yet to be selected and a third state where a winner has been selected and we want to do nothing. We could say that the competition is in three states during this cycle:

• Active
• Inactive
• Closed

How could these three states be handled within the PickWinner method whilst trying to avoid IF statements?

Using the State Pattern

The state pattern is a perfect fit for this scenario. The academic description is that the state pattern will:

Allow an object to alter its behaviour when its internal state changes. The object will appear to change its class.

We start out by creating an interface and a base abstract class. This class defines common methods/properties the three states must implement.

The interface:

public interface ICompetitionState
{
    CompetitionStatus Status { get; }

    void PickWinner(Competition competition);
}

The abstract class:

public abstract class CompetitionState : ICompetitionState
{
    public abstract CompetitionStatus Status { get; }

    public abstract void PickWinner(Competition competition);
}

We then have a class for each state that inherits from this base class:

public class OpenState : CompetitionState
{
    public override CompetitionStatus Status
    {
        get 
        {
            return CompetitionStatus.Open;
        }
    }

    public override void PickWinner(Competition competition)
    {
        if (ClosingDateHasPassed(competition))
        {
            DomainEvents.Raise(new WinnerSelectedEvent(competition, WinnerSelector.GetWinner(competition.Statistics)));
            competition.SetCompetitionState(new ClosedState());
        }
    }

    private static bool ClosingDateHasPassed(Competition competition)
    {
        return (competition.ClosingDate > DateTime.Now);
    }
}

Once the competition has been closed we can now guard it and throw an exception when PickWinner is called when the competition is in a closed state:

public class ClosedState : CompetitionState
{
    public override CompetitionStatus Status
    {
        get
        {
            return CompetitionStatus.Closed;
        }
    }

    public override void PickWinner(Competition competition)
    {
        throw new CompetitionClosedException();
    }
}

In our Competition class we add a property that keeps track of the state which calls the appropriate instance of ICompetitionState.

public class Competition
{
    public CompetitionState State { get; private set; }

    public Competition(CompetitionState state)
    {
        this.State = state;
    }
    
    public void PickWinner()
    {
        this.State.PickWinner();
    }
}

The Open/Closed Principle

Using these classes to represent different states also means that the Competition class conforms to the Open/Closed Principle. The principle states that:

Software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification

In the case of these different states we have done exactly that. If we have a requirement to add a new state we could do so without touching the Competition class. We can extend its functionality without having to modify Competition. Fewer changes = fewer chances to break things!

Testing the State Pattern

In the following test we assert that a winner is selected and the competition state is set to closed. Here is the test:

// Picks the winner, raises the selected winner event and sets the competition status to be closed.
[TestMethod]
public void PickWinnerWithCorrectAnswers()
{
    // Arrange
    Competition competition = Stubs.StubCompetition(includeCorrectAnswers: true);
    OpenState competitionState = new OpenState();
    WinnerSelectedEvent winnerSelectedEvent = null;
    DomainEvents.Register<WinnerSelectedEvent>(evt => winnerSelectedEvent = evt);

    // Act
    competitionState.PickWinner(competition);

    // Assert
    Assert.IsNotNull(winnerSelectedEvent.Competition.Winner);
    Assert.AreEqual(CompetitionStatus.Closed, competition.State.Status);
}

NB Since I've had to write a lot of similar code to build up a Competition class in a particular state I've abstracted this away into a class called Stubs. I use this throughout the tests when I need a stub competition object for testing.

In the next test we assert that an exception is thrown when any subsequent attempt is made to PickWinner when a competition has been closed:

/// Throws exception when pick winner is called on a competition in a closed state.
[TestMethod]
public void PickWinnerWhenACompetitionIsClosedThrowsException()
{
    // Arrange
    Competition competition = Stubs.StubCompetition(includeCorrectAnswers: true);
    ClosedState competitionState = new ClosedState();

    // Act & Assert
    Assert.Throws<CompetitionClosedException>(() => competitionState.PickWinner(competition));
}

I chose to throw an exception rather than just return nothing in this case as this scenario should never happen. It should only happen by way of a developer mistake leaving the door open e.g. by refreshing a browser POST. Code Complete 2 advises:

Throw an exception only for conditions that are truly exceptional.

We are also throwing a custom exception that is consistent with the abstraction we are representing: that the competition is closed and cannot be acted upon.

Regular MSTest users may be wondering where the use of Assert.Throws came from as this neat and tidy way of asserting exceptions doesn't come as standard with MSTest. I made a small library to add this which you can read more about here.

Using Test Stubs

In order to have sufficient code coverage with my unit tests I want to write a unit test for the Competition.PickWinner method. This is tricky, since the method doesn't have an implementation. We have separate tests for each type of state implementation e.g. the OpenState, ClosedState tests we looked at previously. I want to test that Competition.PickWinner calls this.State.PickWinner only once.

This is the method we want to test:

I was able to test this by creating a "stub" class that implements ICompetitionState. I keep an internal count on how many times the method has been called for my test. Here is the code:

In this test we create a simple class called StubCompetitionState that implements the ICompetitionState interface. I added a new property CallCount that we use to track how many times the PickWinner method has been called.

You may have seen the concept of mock objects before, but in this case this would be classed as a stub. For further clarification see the article Mocks Aren't Stubs.

The code for this post can be found at Github.

In the next post we will have a final review of the domain logic before we take a look at how we are going to consume the domain model and make it interact with our application. I've also left some deliberate violations of design principles that will be addressed in the next post. See the RSS subscription links at the bottom of the page to follow this series.