Effective Tests: A Test-First Example – Part 4

Posts In This Series

In part 3 of our Test-First example, we finished the initial implementation of our Tic-tac-toe component. After we finished, a team in charge of creating a host application was able to get everything integrated (though rumor has it that there was a bit of complaining) and the application made its way to the QA team for some acceptance testing. Surprisingly, there were several issues reported that got assigned to us. Here are the issues we’ve been assigned:

As you may have discerned by now, following Test-Driven Development doesn’t ensure the code we produce will be free of errors. It does, however, ensure that our code meets the executable specifications we create to guide the application’s design and aids in the creation of code that’s maintainable and relevant (that is, to the extent we adhere to Test-Driven Development methodologies). Of course, the Test-Driven Development process is a framework into which we pour both our requirements and ourselves. The quality of both of these ingredients certainly affects the overall outcome. As we become better at gathering requirements, translating these requirements into executable specifications, identifying simple solutions and factoring out duplication, our yield from the TDD process will increase.

Since our issues have no particular priority assigned, let’s read through them before we get started to see if any make sense to tackle first. Given a couple of the issues pertain to changes to the API, it might be best to address these first to minimize the possibility of writing new tests that could need to be modified later.

The first of these issues pertain to adding a new method. Here’s the issue:

Upon integrating our component, the GUI team discovered there wasn’t an easy way to tell what positions the game was selecting in order to reflect this to the user. They were able to use techniques similar to those we used in the course of implementing our tests, but they didn’t consider this to be a very friendly API. While such an oversight may seem obvious within the context of the entire application, such issues occur when components are development in isolation. Fundamentally, the problem wasn’t with the Test-Driven Development methodologies we were following, but with the scope in which we were applying them. Later in our series, we’ll discuss an alternative to the approach we’ve taken with this effort that can help avoid misalignments such as this. Until then, we’ll address these issues the best we can with our existing approach.

To address this issue, we’ll create a new test that describes the behavior for the requested method:

[TestClass]
public class When_retrieving_the_last_selected_position_for_the_game
{
  [TestMethod]
  public void it_should_return_the_last_position()
  {
  }
}

As our method of validation, we’ll set up our assertion to verify that some expected position was selected:

[TestClass]
public class When_retrieving_the_last_selected_position_for_the_game
{
  [TestMethod]
  public void it_should_return_the_last_position()
  {
    Assert.AreEqual(1, selection);
  }
}

Next, let’s choose what our API is going to look like and then set up the context of our test. Let’s call our new method GetLastChoiceBy(). We can make use of our existing Player enumeration as the parameter type:

[TestClass]
public class When_retrieving_the_last_selected_position_for_the_game
{
  [TestMethod]
  public void it_should_return_the_last_position()
  {
    Game game = new Game(new GameAdvisorStub(new [] { 1 }));
    game.GoFirst();
    var selection = game.GetLastChoiceBy(Player.Game);
      Assert.AreEqual(1, selection);
  }
}

Next, let’s add the new method to our Game class so this will compile:

public class Game
{
  // snip

  public int GetLastChoiceBy(Player player)
  {
    return 0;
  }
}

Now we’re ready to run the tests:

We can make the test pass by just returning a 1:

public int GetLastChoiceBy(Player player)
{
  return 1;
}

Now, we’ll refactor the method to retrieve the value from a new dictionary field which we’ll set in the SelectAPositionFor() method:

public class Game
{
  readonly Dictionary<Player, char> _tokenAssignments = new Dictionary<Player, char>();
    ...

    void SelectAPositionFor(Player player)
    {
      int recommendedPosition = 
        _advisor.WithLayout(new string(_layout)).SelectBestMoveForPlayer(GetTokenFor(player));
      _layout[recommendedPosition - 1] = GetTokenFor(player);
      _lastPositionDictionary[player] = recommendedPosition;
    }

  public int GetLastChoiceBy(Player player)
  {
    return _lastPositionDictionary[player];
  }
}

That was fairly simple. On to our next feature request:

This is slightly embarrassing. While we’ve been striving to guide the design of our public interface from a consumer’s perspective, we seem to have made a poor choice in how the game communicates errors and the concluding status of the game. If our Game class had evolved within the context of the consuming application, perhaps we would have seen these choices in a different light.

Let’s go ahead and get started on the first part of the request which involves changing how errors are reported. First, let’s take inventory of our existing tests which relate to reporting errors. We have two tests which pertain to error conditions:

[TestClass]
public class When_the_player_attempts_to_select_an_invalid_position
{
  [TestMethod]
  public void it_should_tell_the_player_the_position_is_invalid()
  {
    var game = new Game();
    string message = game.ChoosePosition(10);
    Assert.AreEqual("That spot is invalid!", message);
  }
}

[TestClass]
public class When_the_player_attempts_to_select_an_occupied_position
{
  [TestMethod]
  public void it_should_tell_the_player_the_position_is_occupied()
  {
    var game = new Game(new GameAdvisorStub(new[] { 1, 4, 7 }));
    game.ChoosePosition(2);
    string message = game.ChoosePosition(1);
    Assert.AreEqual("That spot is taken!", message);      
  }
}

Starting with the first method, let’s modify it to check that an exception was thrown:

[TestClass]
public class When_the_player_attempts_to_select_an_invalid_position
{
  [TestMethod]
  public void it_should_tell_the_player_the_position_is_invalid()
  {
    var game = new Game();
    string message = game.ChoosePosition(10);
    Assert.AreEqual("The position '10' was invalid.", exception.Message);
  }
}

Next, let’s wrap the call to the ChoosePosition() method with a try/catch block. We’ll call our exception an InvalidPositionException:

[TestClass]
public class When_the_player_attempts_to_select_an_invalid_position
{
  [TestMethod]
  public void it_should_tell_the_player_the_position_is_invalid()
   {
     var exception = new InvalidPositionException(string.Empty);

       var game = new Game();
     try
     {
       game.ChoosePosition(10);
     }
     catch (InvalidPositionException ex)
     {
       exception = ex;
     }

     Assert.AreEqual("The position '10' was invalid.", exception.Message);
   }
}

Next, let’s create our new Exception class:

public class InvalidPositionException : Exception
{
  public InvalidPositionException(string message) : base(message)
  {
  }
}

Now, let’s run our tests:

Since all we need to do is to throw our new exception, we’ll use an Obvious Implementation:

public string ChoosePosition(int position)
{
  if (IsOutOfRange(position))
  {
    throw new InvalidPositionException(
        string.Format("The position \'{0}\' was invalid.", position));
  }

  if (_layout[position - 1] != '\0')
  {
    return "That spot is taken!";
  }

  _layout[position - 1] = GetTokenFor(Player.Human);
  SelectAPositionFor(Player.Game);

  if (WinningPlayerIs(Player.Human))
    return "Player wins!";

  if (WinningPlayerIs(Player.Game))
    return "Game wins.";

  return string.Empty;
}

In this case we haven’t introduced any duplication that I can see, so let’s move on to our next test. We’ll modify it to follow the same pattern as our previous one:

[TestClass]
public class When_the_player_attempts_to_select_an_occupied_position
{
  [TestMethod]
  public void it_should_tell_the_player_the_position_is_occupied()
  {
    var exception = new OccupiedPositionException(string.Empty);
      var game = new Game(new GameAdvisorStub(new[] {1, 4, 7}));
    game.ChoosePosition(2);

    try
    {
      game.ChoosePosition(1);
    }
    catch (OccupiedPositionException ex)
    {
      exception = ex;
    }

    Assert.AreEqual("The position '1' is already occupied.", exception.Message);
  }
}

Here is our new exception:

public class OccupiedPositionException : Exception
{
  public OccupiedPositionException(string message): base(message)
  {
  }
}

Here’s the results of our test execution:

To provide the implementation, we’ll throw our new exception:

public string ChoosePosition(int position)
{
  if (IsOutOfRange(position))
  {
    throw new InvalidPositionException(string.Format("The position \'{0}\' was invalid.", position));
  }

  if (_layout[position - 1] != '\0')
  {
    throw new OccupiedPositionException(
        string.Format("The position \'{0}\' is already occupied.", position));
  }

  _layout[position - 1] = GetTokenFor(Player.Human);
  SelectAPositionFor(Player.Game);

  if (WinningPlayerIs(Player.Human))
    return "Player wins!";

  if (WinningPlayerIs(Player.Game))
    return "Game wins.";

  return string.Empty;
}

Again, there isn’t any noticeable duplication this time. Our next task is to send notifications to observers when the Game class detects a winner. Here are the existing tests we used for indicating a winner:

[TestClass]
public class When_the_player_gets_three_in_a_row
{
  [TestMethod]
  public void it_should_announce_the_player_as_the_winner()
  {
    var game = new Game(new GameAdvisorStub(new[] {1, 2, 3}));
    game.ChoosePosition(4);
    game.ChoosePosition(5);
    string message = game.ChoosePosition(6);
    Assert.AreEqual("Player wins!", message);
  }
}

[TestClass]
public class When_the_game_gets_three_in_a_row
{
  [TestMethod]
  public void it_should_announce_the_game_as_the_winner()
  {
    var game = new Game();
    game.ChoosePosition(4);
    game.ChoosePosition(6);
    string message = game.ChoosePosition(8);
    Assert.AreEqual("Game wins.", message);
  }
}

We’ll start with the first one by modifying our Assert call. First, let’s change the value we’re comparing against from a string to a new GameResult enumeration with a value of PlayerWins:

[TestClass]
public class When_the_player_gets_three_in_a_row
{
  [TestMethod]
  public void it_should_announce_the_player_as_the_winner()
  {
    var game = new Game(new GameAdvisorStub(new[] {1, 2, 3}));
    game.ChoosePosition(4);
    game.ChoosePosition(5);
    string message = game.ChoosePosition(6);
    Assert.AreEqual(GameResult.PlayerWins, result);
  }
}

Next, let’s create an instance of our as yet created GameResult enumeration and initialize it’s value to something we aren’t expecting:

[TestClass]
public class When_the_player_gets_three_in_a_row
{
  [TestMethod]
  public void it_should_announce_the_player_as_the_winner()
  {
    var game = new Game(new GameAdvisorStub(new[] {1, 2, 3}));
    var result = (GameResult) (-1);
    game.ChoosePosition(4);
    game.ChoosePosition(5);
    string message = game.ChoosePosition(6);
    Assert.AreEqual(GameResult.PlayerWins, result); 
  }
}

Next, we need to decide how we would like to receive this value. Let’s assume we can subscribe to a GameComplete event. When invoked, we’ll assume the value can be retrieved from a property on the EventArgs supplied with the event:

[TestClass]
public class When_the_player_gets_three_in_a_row
{
  [TestMethod]
  public void it_should_announce_the_player_as_the_winner()
  {
    var game = new Game(new GameAdvisorStub(new[] {1, 2, 3}));
    var result = (GameResult) (-1);
    game.GameComplete += (s, e) => result = e.Result;
    game.ChoosePosition(4);
    game.ChoosePosition(5);
    game.ChoosePosition(6);
    Assert.AreEqual(GameResult.PlayerWins, result);
  }
}

Our next steps are to create the new enum type and to add an event to our Game class. First, let’s create the enum:

public enum GameResult
{
  PlayerWins,
  GameWins,
  Draw
}

I went ahead and added values for the other two possible states: GameWins and Draw. “Aren’t we getting ahead of ourselves”, you might ask? Perhaps, but we already know we have upcoming tests that will require these states and our GameResult represents the state of our game, not its behavior. We’ve been pretty good about not prematurely adding anything thus far, so this seems like a safe enough step to take without sending us down a slippery slope.

Here’s our new Game event:

public class Game
{
  ...

    public event EventHandler<GameCompleteEventArgs> GameComplete;

    ...
}

Now that we’ve created this, we’ll also need to create a GameCompleteEventArgs:

public class GameCompleteEventArgs : EventArgs
{
  public GameResult Result { get; private set; }
}

Now we’re ready to compile and run our tests:

There are well established patterns for raising events in .Net, so we’ll follow the standard pattern for this:

public string ChoosePosition(int position)
{
  if (IsOutOfRange(position))
  {
    throw new InvalidPositionException(string.Format("The position \'{0}\' was invalid.", position));
  }

  if (_layout[position - 1] != '\0')
  {
    throw new OccupiedPositionException(string.Format("The position \'{0}\' is already occupied.", position));
  }

  _layout[position - 1] = GetTokenFor(Player.Human);
  SelectAPositionFor(Player.Game);

  if (WinningPlayerIs(Player.Human))
    InvokeGameComplete(new GameCompleteEventArgs(GameResult.PlayerWins));

      if (WinningPlayerIs(Player.Game))
        return "Game wins.";

  return string.Empty;
}

Now we need to create our InvokeGameComplete() method and a GameCompleteEventArgs constructor that initializes the Result property:

public class Game
{
  ... 

    public event EventHandler<GameCompleteEventArgs> GameComplete;

  public void InvokeGameComplete(GameCompleteEventArgs e)
  {
    EventHandler<GameCompleteEventArgs> handler = GameComplete;
    if (handler != null) handler(this, e);
  }

  ... 

}

public class GameCompleteEventArgs : EventArgs
{
  public GameCompleteEventArgs(GameResult result)
  {
    Result = result;
  }

  public GameResult Result { get; private set; }
}

Again, I don’t see any duplication to worry about. Next, we’ll follow similar steps for notifying the game as a winner:

[TestClass]
public class When_the_game_gets_three_in_a_row
{
  [TestMethod]
  public void it_should_announce_the_game_as_the_winner()
  {
    var game = new Game(new GameAdvisorStub(new[] {1, 2, 3}));
    var result = (GameResult) (-1);
    game.GameComplete += (s, e) => result = e.Result;
    game.ChoosePosition(4);
    game.ChoosePosition(6);
    game.ChoosePosition(8);
    Assert.AreEqual(GameResult.GameWins, result);
  }
}

To make the test pass, we should only need to modify the Game class to raise the event this time:

public string ChoosePosition(int position)
{
  if (IsOutOfRange(position))
  {
    throw new InvalidPositionException(string.Format("The position \'{0}\' was invalid.", position));
  }

  if (_layout[position - 1] != '\0')
  {
    throw new OccupiedPositionException(string.Format("The position \'{0}\' is already occupied.", position));
  }

  _layout[position - 1] = GetTokenFor(Player.Human);
  SelectAPositionFor(Player.Game);

  if (WinningPlayerIs(Player.Human))
    InvokeGameComplete(new GameCompleteEventArgs(GameResult.PlayerWins));

  if (WinningPlayerIs(Player.Game))
    InvokeGameComplete(new GameCompleteEventArgs(GameResult.GameWins));

      return string.Empty;
}

Let’s run the tests again:

Our target test passed, but we broke our previous test. Looking at our implementation, the problem seems to be that both the player and game select positions on the board before we check to see if anyone is a winner. Additionally, we should return from the method once a winner is determined. Let’s fix this:

public string ChoosePosition(int position)
{
  if (IsOutOfRange(position))
  {
    throw new InvalidPositionException(string.Format("The position \'{0}\' was invalid.", position));
  }

  if (_layout[position - 1] != '\0')
  {
    throw new OccupiedPositionException(string.Format("The position \'{0}\' is already occupied.", position));
  }

  _layout[position - 1] = GetTokenFor(Player.Human);

  if (WinningPlayerIs(Player.Human))
  {
    InvokeGameComplete(new GameCompleteEventArgs(GameResult.PlayerWins));
    return string.Empty;
  }

  SelectAPositionFor(Player.Game);

  if (WinningPlayerIs(Player.Game))
  {
    InvokeGameComplete(new GameCompleteEventArgs(GameResult.GameWins));
    return string.Empty;
  }

  return string.Empty;
}

Let’s refactor now. First, let’s remove our unused return type:

public void ChoosePosition(int position)
{
  if (IsOutOfRange(position))
  {
    throw new InvalidPositionException(string.Format("The position \'{0}\' was invalid.", position));
  }

  if (_layout[position - 1] != '\0')
  {
    throw new OccupiedPositionException(string.Format("The position \'{0}\' is already occupied.", position));
  }

  _layout[position - 1] = GetTokenFor(Player.Human);

  if (WinningPlayerIs(Player.Human))
  {
    InvokeGameComplete(new GameCompleteEventArgs(GameResult.PlayerWins));
    return;
  }


  SelectAPositionFor(Player.Game);

  if (WinningPlayerIs(Player.Game))
  {
    InvokeGameComplete(new GameCompleteEventArgs(GameResult.GameWins));
    return;
  }
}

Now that we’ve rearranged our code, we have a sequence of steps that are repeated between the player and the game. First we use a strategy for moving the player, then we check to see if the player wins. Let’s distill this down to checking for the first winning play from a collection of player strategies:

public void ChoosePosition(int position)
{
  if (IsOutOfRange(position))
  {
    throw new InvalidPositionException(string.Format("The position \'{0}\' was invalid.", position));
  }

  if (_layout[position - 1] != '\0')
  {
    throw new OccupiedPositionException(string.Format("The position \'{0}\' is already occupied.", position));
  }

  new Func<bool>[]
  {
    () => CheckPlayerStrategy(Player.Human, () => _layout[position - 1] = GetTokenFor(Player.Human)),
      () => CheckPlayerStrategy(Player.Game, () => SelectAPositionFor(Player.Game))
  }.Any(winningPlay => winningPlay());
}

Here’s our new CheckPlayerStrategy() method:

bool CheckPlayerStrategy(Player player, Action strategy)
{
  strategy();
  if (WinningPlayerIs(player))
  {
    var result = (player == Player.Human) ? GameResult.PlayerWins : GameResult.GameWins;
    InvokeGameComplete(new GameCompleteEventArgs(result));
    return true;
  }

  return false;
}

Our final step for this issue is to raise an event when there is a draw. Following our normal procession, here’s the test we come up with:

[TestClass]
public class When_a_move_results_in_a_draw
{
  [TestMethod]
  public void it_should_announce_the_game_is_a_draw()
  {
    var game = new Game(new GameAdvisorStub(new[] { 2, 3, 4, 9 }));
    var result = (GameResult)(-1);
    game.GameComplete += (s, e) => result = e.Result;
    new[] {1, 5, 6, 7, 8}.ToList().ForEach(game.ChoosePosition);
    Assert.AreEqual(GameResult.Draw, result);
  }
}

This test isn’t failing for the right reason, so let’s address this before moving on. After investigating the exception, the issue is that we never accounted for the fact that their won’t be a position to choose when the player chooses the last remaining position. Let’s correct this issue by ensuring there is an empty spot left before selecting a position for the game:

void SelectAPositionFor(Player player)
{
  if (_layout.Any(position => position == '\0'))
  {
    int recommendedPosition =
      _advisor.WithLayout(new string(_layout)).SelectBestMoveForPlayer(GetTokenFor(player));
    _layout[recommendedPosition - 1] = GetTokenFor(player);
    _lastPositionDictionary[player] = recommendedPosition;
  }
}

Now our test is failing for the right reason. To make the test pass, we can fire the event unless someone won or unless there’s any empty positions left:

public void ChoosePosition(int position)
{
  if (IsOutOfRange(position))
  {
    throw new InvalidPositionException(string.Format("The position \'{0}\' was invalid.", position));
  }

  if (_layout[position - 1] != '\0')
  {
    throw new OccupiedPositionException(string.Format("The position \'{0}\' is already occupied.", position));
  }

  var someoneWon = new Func<bool>[]
  {
    () => CheckPlayerStrategy(Player.Human, () => _layout[position - 1] = GetTokenFor(Player.Human)),
      () => CheckPlayerStrategy(Player.Game, () => SelectAPositionFor(Player.Game))
  }.Any(winningPlay => winningPlay());

  if (!(someoneWon || _layout.Any(pos => pos == '\0')))
  {
    InvokeGameComplete(new GameCompleteEventArgs(GameResult.Draw));
  }
}

Time to refactor. It looks like we have the same comparison for checking that the game is a draw and our new guard for selecting a position for the game. Let’s create a method for these which expresses the meaning of this check:

bool PositionsAreLeft()
{
  return _layout.Any(pos => pos == '\0');
}

Now we can replace the previous calls in the ChoosePosition() and SelectAPositionFor() methods:

public void ChoosePosition(int position)
{
  if (IsOutOfRange(position))
  {
    throw new InvalidPositionException(string.Format("The position \'{0}\' was invalid.", position));
  }

  if (_layout[position - 1] != '\0')
  {
    throw new OccupiedPositionException(string.Format("The position \'{0}\' is already occupied.", position));
  }

  bool someoneWon = new Func<<bool>[]
  {
    () =>
      CheckPlayerStrategy(Player.Human,
          () => _layout[position - 1] = GetTokenFor(Player.Human)),
      () => CheckPlayerStrategy(Player.Game, () => SelectAPositionFor(Player.Game))
  }.Any(winningPlay => winningPlay());

  if (!(someoneWon || PositionsAreLeft()))
  {
    InvokeGameComplete(new GameCompleteEventArgs(GameResult.Draw));
  }
}

void SelectAPositionFor(Player player)
{
  if (PositionsAreLeft())
  {
    int recommendedPosition =
      _advisor.WithLayout(new string(_layout)).SelectBestMoveForPlayer(GetTokenFor(player));
    _layout[recommendedPosition - 1] = GetTokenFor(player);
    _lastPositionDictionary[player] = recommendedPosition;
  }
}

One thing that occurred to me while implementing this feature is that using a null character to represent an empty position isn’t particularly clear. Let’s define a constant named EmptyValue which we’ll substitute for our use of the null character:

public class Game
{
  ...

    const char EmptyValue = char.MinValue;

    ...

    public void ChoosePosition(int position)
    {
      ...

        if (_layout[position - 1] != EmptyValue)
        {
          throw new OccupiedPositionException(string.Format("The position \'{0}\' is already occupied.", position));
        }

      ...
    }

  bool PositionsAreLeft()
  {
    return _layout.Any(pos => pos == EmptyValue );
  }

  string GetLayoutFor(Player player)
  {
    return new string(_layout.ToList()
        .Select(c => (c.Equals(GetTokenFor(player))) ? GetTokenFor(player) : EmptyValue )
        .ToArray());
  }

  …
}

That wraps up the two issues from the UI team. We’ll stop here and address the issues from the QA team next time.