Monday, February 28, 2011

Design contradictions between test levels

I thought I'd spend tonight finishing off the BDD Chessboard Kata exercise I started with Jim McDonald at an XP Manchester coding dojo. Jim is a fan of emergent design, whereas I advocate a little design up front.
For those who don't know the kata. There are a number of predefined specifications written in the Gherkin DSL based around the movement of 2 chess pieces on a chessboard.
This is the feature we were implementing.

Feature: Boundaries of the board.
In order to obey the rules of Chess
As a player
I want to be prevented from entering moves outside the boundary of the board.

Scenario: Pawn at top.
Given I have a White Pawn at A8
And I have a Black Knight at A1
When I move the Pawn to A9
Then I should be warned of an illegal move message

Scenario: Knight heads off board
Given I have a Black Knight at G8
And I have a White Pawn at A1
And I move the Pawn to A2
When I move the Knight to I7
Then I should be warned of an illegal move message
We've completed most of the steps on each of the scenarios using a location class implementing


    public interface ILocation
    {
        void MoveTo(string location);
    }

Each time we touched a class we've covered it with mspec tests.

    [Subject(typeof(Location))]
    public class When_creating_a_location_with_an_invalid_position
    {

        Because of = () =>; exception = Catch.Exception(() =>; new Location("Z22"));

        Behaves_like<AnInvalidLocation>; its_in_the_wrong_location;

        protected static Exception exception;
    }

    [Subject(typeof(Location))]
    public class When_moving_a_piece_to_invalid_location
    {
        Establish that = () =>; location = new Location("A1");
        Because of = () =>; exception = Catch.Exception( ()=>; location.MoveTo("X2"));
        Behaves_like<AnInvalidLocation> an_invalid_move; 

        protected static Exception exception;
        static ILocation location;
    }
I've DRYed out (a little) the invalid move behaviour in my Location.

Now that I've come to implement my final step, "Then I should be warned of an illegal move message".
I suddenly need to implement the invalid move behaviour. (Even though the WHEN is where the invalid move occurs I don't need to write the behaviour yet).

So this is where the emergent design has lead me to. If I am to follow the YAGNI or LRM principles I don't want do anything thing more than display a message. But If I change the behaviour of my location classes' MoveTo method to return a message, then the good design principles that my class level specifications has lead me too are at odds with the demands of the specification passing.

I am trying not to introduce another class to handle game messaging but by not doing so I undo my hard work at the class level. Is this the last responsible moment?



2 comments:

Jason Imison said...

Why is the black knight at A1 relevant?

Johnno Nolan said...

Good question. Because the board is in a specific state I thought it would be useful to specify the Black piece. In the pawn illegal move scenario it is arguably unnecessary as the position of the white piece is at the boundary of the board.

At better way to write the cuke may have been

GIVEN the pieces are in the following positions
|Piece|Location|
|Pawn|A8|
|Knight|A1|

or

https://gist.github.com/796009

I used a more laboured specification because at the UI level i was taking moves.

E.g.

Game.MovePawn('A8');
Game.MoveKnight('A1');