Refactoring phase of the TDD Traffic Light - how to get this right?

Question

So I made the following test for a class Board that would to be born:

[TestMethod]
public void Set_The_Origin_As_Violet_And_The_Query_Confirms_It() {
    Board board = new Board(10, 10);

    Color expected = Color.Violet;
    board.SetColorAt(0, 0, expected);
    Color actual = board.GetColorAt(0, 0);
    Assert.AreEqual(expected, actual);
}

I tried running the code, but the compiler signaled that Board didn't exist. So I created it.

I tried to run the code again, but it was of no avail, as both SetColorAt() and GetColorAt() methods didn't exist. I created them:

public void SetColorAt(int x, int y, Color color) {
}

public void GetColorAt(int x, int y) {
}

Still, not everything was fine, as I had to return Color.Violet. So I changed GetColorAt() to

public void GetColorAt(int x, int y) {
    return Color.Violet;
}

So I got a green light for the first time.

What I want my final code to be on the class board is something of the form:

public class Board
{
    private Color[,] board;

    public Board(int x, int y)
    {
        board = new Color[x, y];
    }

    public void SetColorAt(int x, int y, Color color) {
        board[x, y] = color;
    }

    public Color GetColorAt(int x, int y) {
        return board[x, y];
    }
}

My first question is....

how to get there? Can I consider that in the "refactor" phase of the Unit-Test I have shown above, when removing duplication I will eventually end up with this code?

If the answer is yes, I feel that my Unit-test is testing something way too "localized" for what the code actually does. You see, the test is just checking for 1 pixel and one color, while the code itself is way more complex and rich.

Maybe the solution would be to add more Unit-Tests? Which would you advise to make?

My second question is...

I know later I'll want to have an IBoard. Should I just express that in the above Unit-Test? Should I let

Board board = new Board(10, 10);

as it is and still create the IBoard interface? How to deal with this?

Answer 1

public void GetColorAt(int x, int y) {
    return Color.Violet;
}

The return Color.Violet statement here can be considered Data duplication, so you can refactor this bit out. The simplest thing that could possible work though would be having a single Color value that gets set when you call SetColorAt

public class Board
{
    private Color theColor;

    public Board(int x, int y)
    {
    }

    public void SetColorAt(int x, int y, Color color) {
        theColor = color;
    }

    public Color GetColorAt(int x, int y) {
        return theColor;
    }
}

Now, you'll need more tests, to show you can set seperate cells to different colors.

Answer 2

You're familiar with the test pattern Arrange-Act-Assert, right? That's what your test does.

[TestMethod]
public void Set_The_Origin_As_Violet_And_The_Query_Confirms_It() {
    // Arrange
    Board board = new Board(10, 10);
    // Act
    board.SetColorAt(0, 0, expected);
    // Assert
    Color expected = Color.Violet;
    Assert.AreEqual(expected, board.GetColorAt(0, 0));
}

I like to use a slightly modified form, Arrange-AssertNot-Act-Assert. The idea is that we verify that the Act itself is what brings about the condition for which we are testing, because we assert before the act that our condition is not met. Here's how that would look:

[TestMethod]
public void Set_The_Origin_As_Violet_And_The_Query_Confirms_It() {
    // Arrange
    Board board = new Board(10, 10);
    // Assert
    Color expected = Color.Violet;
    Assert.AreNotEqual(expected, board.GetColorAt(0, 0));
    // Act
    board.SetColorAt(0, 0, expected);
    // Assert
    Assert.AreEqual(expected, board.GetColorAt(0, 0));
}

This compels you to write a slightly less simple implementation the first time around. You will still need additional tests to drive a full implementation of GetColorAt(); for this I like to feel that the time to write the full implementation is when it's less hassle than just faking in another special case (somewhere around N=3, I would guess, for this app).

Answer 3

Start with a high level description of what you want to implement.

• A square board with rows and columns.
• Each board position should have a color.
• A board user should be able to set the color of a specific board position (pixel).
• A board user should be able to get the color of a specific board position.
• etc.

Next, use small tests to drive out the implementation. Sometimes it is easier to start with boundary conditions.

When asked to get the color at a specific board position
- given a default board, and
  - the row value is less than zero
  - should throw argument exception

When asked to get the color at a specific board position
- given a default board, and
  - the row value is greater than the highest board row
  - should throw argument exception

"the row value is greater than the highest board row" forces you to have some way to set the highest board row. The test doesn't care whether you set it via the constructor or a property setter.

When constructed with board dimensions
 - should set the highest board row

"should set the highest board row" forces you to have some way to access the value that was set. Perhaps a property getter.

When asked to get the color at a specific board position
- given a default board, and
  - the column value is less than zero
  - should throw argument exception

When asked to get the color at a specific board position
- given a default board, and
  - the column value is greater than the highest board column
  - should throw argument exception

"the column value is greater than the highest board column" forces you to duplicate the highest row solution for the highest column.

When constructed with board dimensions
 - should set the highest board column

etc.

When asked to get the color at a specific board position
- given a default board, and
  - the row value is within the board space, and
  - the column is within the board space
  - should return the default color

"should return the default color" forces you to expose the default color somewhere. This shouldn't be a magic value that only the test knows about. Expose it as a const or read only value on Board.

When asked to get the color at a specific board position
- given a default board, and
  - the row value is within the board space, and
  - the column is within the board space, and
  - the requested position has a known color
  - should return the color of the board at the requested position

"the requested position has a known color" forces you to build out the ability to set the color of a board position. You can do that now because you have a failing test for getting the color at a position. So set that aside and build out the setter.

When asked to SET the color at a specific board position
- given a default board, and
  - the row value is less than zero
  - should throw argument exception

When asked to set the color at a specific board position
- given a default board, and
  - the row value is greater than the highest board row
  - should throw argument exception

When asked to set the color at a specific board position
- given a default board, and
  - the column value is less than zero
  - should throw argument exception

When asked to set the color at a specific board position
- given a default board, and
  - the column value is greater than the highest board column
  - should throw argument exception

When asked to set the color at a specific board position
- given a default board, and
  - the row value is within the board space, and
  - the column is within the board space
  - should set the color at the given position to the requested color

"should set the color at the given position to the requested color" uses your get implementation to verify the value was set correctly. Once you've implemented it correctly all your tests will be green again.

Just continue to build out additional functionality one small piece at a time.

When you approach the tests in implementation-agnostic terms like this you can change the implementation to use IBoard or Ruby or whatever and the test descriptions don't have to change. You'll have to change some test implementation details but the stated goals of the tests don't have to change.

Answer 4

I think you jumped the gun with writing a board that sets a color and gets a color. What drove a board? What requires a board to store colors? The board is a repository for colors?

The board has too much implementation details. IOWs, the color is a property of another object. Do you really care that the board stores colors or does it store this domain object that has a color property. What if later you have too store the past history of the board positions. Are you going then to use two boards, one a color board and then a history board.

You really need to abstract the storage mechanism away from actually relying on specific types of color (purple, blue, green, etc...).

The problem with starting off with an understanding of the problem domain is that it will pigeon hole your design. And it will not be so robust.

For me I would have started at a question: What domain object is color a property of? Then my tests would have generated this domain object. For all I know The domain object may or may not exist in a board.

Hope this helps you.