Archive for the ‘TDD’ Category

Painless TDD – Strip away your dependencies!

Monday, May 14th, 2012

Have you ever come across the problem where, in order to construct the object you are trying to test, you end up creating massive scaffolding code just to pass to the constructor? Perhaps an example is in order:

class ChessGame {
    // Some instance variables
 
    public ChessGame (Player player1, Player player2, int timeLimit) {
        this.player1 = player1;
        this.player2 = player2;
        this.timeLimit = timeLimit;
    }
 
    public void playGame() {...}
}
 
class ChessGameTest {
    @Test
    public void testChessGamePlaysForXseconds () throws Exception {
        Player player1 = createMock(Player.class);
        Player player2 = createMock(Player.class);
 
        Handicap handicap1 = new Handicap(10, Rules.Standard);
        Handicap handicap2 = new Handicap(4,  Rules.Standard);
 
        expect(player1.getName()).andReturn("D. Duck");
        expect(player2.getName()).andReturn("M. Mouse");
 
        expect(player1.getHandicap()).andReturn(handicap1);
        expect(player2.getHandicap()).andReturn(handicap2);
 
        replayAll();
 
        new ChessGame(player1, player2, 10).playGame();
 
        // Assertions ....
    }
}

The problem here is that we’ve specified dependencies on far too many objects and values! By asking for two Player objects, you’re essentially saying “give me two objects which have names, ages, total scores, rankings, biographies, etc… and which also have very detailed Handicap information”.

Ask yourself this question:

  • Without looking inside the class file, what exactly does the ChessGame need in order to execute playGame()?

It’s not at all obvious is it! My guess would be that we only really need player names, and handicap information (why do we need the player bio to play a match??). If this is the case, why do we pass all that extra information to the ChessGame constructor, if it’s never going to need it?

I can hear you answering “But it’s far more convenient to package up that information as it exists naturally – inside the Player object. It is more readable, and keeps the number of constructor parameters down!”

But this is not a valid reason because of the following:

  1. Classes should not ask for more information than they need – otherwise they become difficult and cumbersome to test
  2. Classes should make absolutely clear what their dependencies are. This is why passing around “Contexts” is a bad thing

We can make this a lot easier by using a Factory Method as a convenience:

class ChessGame {
 
    public ChessGame (String player1Name, String player2Name, 
                int player1Handicap, int player2Handicap, int timeLimit) {
 
        this.player1Name = player1Name;
        this.player2Name = player2Name;	
        //..etc
    }
 
    public static ChessGame withPlayers (Player player1, Player player2, int timeLimit) {
        return new ChessGame (
            player1.getName(), 
            player2.getName(),
            player1.getHandicap().getAbsolute(),
            player2.getHandicap().getAbsolute());
    }
 
    public void playGame() {...}
}
 
class ChessGameTest {
    @Test
    public void testChessGamePlaysForXseconds () throws Exception {
 
        new ChessGame("D. Duck", "M. Mouse", 10, 4, 10);
 
        // Assertions ....
    }
}

The second ChessGame class accepts only what it needs – it has fewer dependencies on real objects. However, we now have a nice convenience method on the object which takes our existing objects.

Why stop there? Here are some more Factory Methods which you might prove useful:

public static ChessGame fromExistingGame (ChessGame previousGame) {...}
public static ChessGame againstBot (BotFactory botFactory) {...}
public static ChessGame fromDataFile (File dataFile) {...}

 

So in conclusion – if you’re finding yourself setting up tonnes of scaffolding for a test, ask yourself whether the unit-to-be-tested is asking too much of you!

Painless TDD – Using Equality to Test Your Code

Monday, May 7th, 2012

… or, why Getters are a Code Smell!

A common problem arises when you need to change the underlying implementation of a class, and you want to use your existing tests to assert that the outward behaviour of this class has not changed. You may well take the following approach:

  1. Make changes to your class implementation
  2. Fix any tests that have broken
  3. Re-run the tests. Do they pass?

Can you spot a problem with this approach? When you change your tests, you cannot rely on them to validate your code anymore. The point of verifying change using Unit Tests is that they remain an invariant. If you have changed both your code and your tests, then you have no confidence that you haven’t introduced a bug in your work!

Look at the code sample below:

    @Test
    public void testTwoCorrectAnagramsAreGenerated() throws Exception{
 
        // Given : a basic anagram generator
        // When : presented with the word "sleep"
        WordList wordList = new AnagramGenerator().generateAnagrams("sleep");
 
        // Then : we get two anagrams - "sleep" and "peels"
        List words = wordList.getWords();
        assertEquals("First anagram", "sleep", words.get(0));
        assertEquals("Second anagram", "peels", words.get(1));
        assertEquals("Size", 2, words.size());
    }

This simple test is flawed:

  1. We are assuming that the implementation of WordList will always store its words in a List<String>. In fact, we are forced to expose the underlying structure of our object.
  2. The test will fail if we change the order in which the anagrams are returned. Should the order of the returned anagrams really be a concern in this situation?
  3. WordList has become a central focal point of our test, even though we are testing the AnagramGenerator
A better test would instead use the .equals() method to test correctness:
    @Test
    public void testTwoCorrectAnagramsAreGenerated() throws Exception{
 
        // Given : a basic anagram generator
        // When : presented with the word "sleep"
        WordList list = new AnagramGenerator().generateAnagrams("sleep");
 
        // Then : we get two anagrams - "sleep" and "peels"
        assertEquals("Anagram List", new WordList("sleep","peels"), list);
   }

The result is that we have a smaller test, which doesn’t poke around inside the WordList class when making assertions. The only thing we need to assume is that Object.equals(Object) has been implemented correctly (testing this can be done separately in WordListTest).

Furthermore, if we decide that ordering of results is not important, we can have WordList implement a HashSet. The .equals() method will therefore not check for ordering.

Using equality rather than introspection will ensure that your tests are less brittle and invariant to change. Happy TDDing!