… 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

    @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!

Tags: Code Smell, Java, JUnit, Refactoring, TDD, Unit Testing

This entry was posted on Monday, May 7th, 2012 at 2:35 pm and is filed under TDD. You can follow any responses to this entry through the RSS 2.0 feed. You can leave a response, or trackback from your own site.