January 30, 2005

Replacing Inheritance with Composition

One way to remove duplication is using inheritance. However, in many cases composition is better and just as easy to implement.

Consider the following code:


public class UpperCaseEmphasiser {
    public String emphasise(String something) {
        StringBuffer result = new StringBuffer();
        StringTokenizer tokens = new StringTokenizer(something, " ", true);
        while(tokens.hasMoreTokens()){
            String token = tokens.nextToken();
            if(!token.equals(" ")){
                token = token.toUpperCase();
            }
            result.append(token);
        }
        return result.toString();
    }
}

with the following tests:


import junit.framework.TestCase;

public class UpperCaseEmphasiserTest extends TestCase {
    private UpperCaseEmphasiser emphasiser = new UpperCaseEmphasiser();
    
    public void testThatEmphasiserDoesNotPutAnyEmphasisOnEmptyString() {
        assertEquals("", emphasiser.emphasise(""));
    }
    public void testThatEmphasiserPutsEmphasisOnOneWord() {
        assertEquals("HELLO", emphasiser.emphasise("hello"));
    }
    public void testThatEmphasiserPutsEmphasisOnMultipleWords() {
        assertEquals("HELLO WORLD", emphasiser.emphasise("hello world"));
    }
}

and another class called "AsteriskEmphasiser", which is exactly the same except having "token = ""token"";" instead of "token = token.toUpperCase();" and the tests for it being slightly different as appropriate.

Using inheritance to remove duplication

There is a lot of duplication between "UpperCaseEmphasiser" and "AsteriskEmphasiser" and also between their tests. One way this duplication can be removed is using inheritance, creating an abstract class for the duplicated code and having the concrete subclasses containing only the code that is different:


public abstract class Emphasiser {
    public String emphasise(String something) {
        StringBuffer result = new StringBuffer();
        StringTokenizer tokens = new StringTokenizer(something, " ", true);
        while(tokens.hasMoreTokens()){
            String token = tokens.nextToken();
            if(!token.equals(" ")){
                token = emphasiseWord(token);
            }
            result.append(token);
        }
        return result.toString();
    }

    abstract String emphasiseWord(String token);
}

and then UpperCaseEmphasiser becomes:


public class UpperCaseEmphasiser extends Emphasiser {
    String emphasiseWord(String token) {
        return token.toUpperCase();
    }
}

and "AsteriskEmphasiser" is modified similarly.

Using composition instead of inheritance

Using inheritance has eliminated the duplication between the classes, but not between the tests. Sometimes, people will use inheritance in the tests to eliminate that duplication too, however, I suggest that using composition instead is better, because it separates the code into the functionality that is shared and the functionality that is different, in a way in which the functionality that is different can be called independently of the code that is shared. In this example, Emphasiser can be modified as:


public class Emphasiser {
    private WordEmphasiser wordEmphasiser;
    
    public Emphasiser(WordEmphasiser wordEmphasiser){
        this.wordEmphasiser = wordEmphasiser;
    }

    private String emphasiseWord(String token){
        return wordEmphasiser.emphasiseWord(token);
    }
    
    public String emphasise(String something) {
    	... unchanged ...

and UpperCaseEmphasiser becomes:


public class UpperCaseEmphasiser implements WordEmphasiser {
    public String emphasiseWord(String token) {
        return token.toUpperCase();
    }
}

(and WordEmphasiser is:


public interface WordEmphasiser {
    String emphasiseWord(String token);
}

)

and the tests can be kept the same by doing:


public class UpperCaseEmphasiserTest extends TestCase {
    private Emphasiser emphasiser = new Emphasiser(new UpperCaseEmphasiser());
        ... unchanged ...

The benefit of composition

The code as shown so far isn't a big change from using inheritance, but does have a big effect. With this arrangement, instead of having the tests duplicated (or using inheritance in the tests to remove the duplication) the tests can now be split into two parts. One test for the class with the code that is common, and separate tests for the code that is different. The tests for the common code now becomes:


public class EmphasiserTest extends TestCase {
    private MockWordEmphasiser aMockWordEmphasiser = new MockWordEmphasiser();
    class MockWordEmphasiser implements WordEmphasiser {
        public Stack expectedWords = new Stack();
        public String emphasiseWord(String word) {
            String expectedWord = (String)expectedWords.pop();
            assertEquals(expectedWord, word);
            return word;
        }        
    }
    
    private Emphasiser emphasiser = new Emphasiser(aMockWordEmphasiser);
    
    public void testThatEmphasiserDoesNotPutAnyEmphasisOnEmptyString() {
        Object ensureEmphasiseWordIsNotCalled = new Object();
        aMockWordEmphasiser.expectedWords.add(ensureEmphasiseWordIsNotCalled);
        assertEquals("", emphasiser.emphasise(""));
    }
    public void testThatEmphasiserPutsEmphasisOnOneWord() {
        aMockWordEmphasiser.expectedWords.add("hello");
        assertEquals("hello", emphasiser.emphasise("hello"));
    }
    public void testThatEmphasiserPutsEmphasisOnMultipleWords() {
        aMockWordEmphasiser.expectedWords.add(0,"hello");
        aMockWordEmphasiser.expectedWords.add(0,"world");
        assertEquals("hello world", emphasiser.emphasise("hello world"));
    }
}

which is completely independent of the implementations of any "WordEmphasiser". There are many other ways to implement "Mock" objects, but that is beyond the scope of this article. Here I've presented a simple implementation which I hope is clear as it does not need any knowledge or reference to anything outside of the code presented.

Note that now the test is testing that the "WordEmphasiser" is called with the correct arguments and the "Emphasiser" returns the words converted using whatever the "WordEmphasiser" returns (in this case, making no change to the words).

The tests for the "UpperCaseEmphasiserTest" are now very simple:


public class UpperCaseEmphasiserTest extends TestCase {
    private WordEmphasiser emphasiser = new UpperCaseEmphasiser();
    
    public void testThatEmphasiserPutsEmphasisOnOneWord() {
        assertEquals("HELLO", emphasiser.emphasiseWord("hello"));
    }
}

and similarly for "AsteriskEmphasiserTest". Note that now one test is sufficient, because the WordEmphasiser is only called for one word at a time - it isn't called for spaces or multiple words, because that is taken care of by the "Emphasiser". This is an example of a "separation of concerns"; i.e. the splitting of a string into words has been separated from the application of emphasis to each word. There are other benefits in addition to the simplification of the tests, and the removal of duplication. Using composition also allows more flexibility in combining these classes together. For example, there could be an "Emphasiser" which puts emphasis on every other word, which could reuse the "WordEmphasisers" independently of thier use by the standard "Emphasiser".

Whenever you see inheritance, particularly tests using inheritance, consider whether converting to composition would allow better separatation of concerns, making the tests clearer and the code cleaner.

Posted by ivan at January 30, 2005 11:13 PM
Copyright (c) 2004-2008 Ivan Moore
Comments

A thought provoking post Ivan.

It's interesting that if you set out to eliminate test duplication of the sort you describe via test refactoring, you will tend to introduce delegation. Whether or not delegation or inheritance is appropriate is doubtless dependent on context. At the very least test duplication is an indicator that you have a decision to make. I'll be thinking now about the sorts of indicative duplication that delegation causes in tests and how to refactor gracefully between the two design choices.

One comment though - you've used the Strategy pattern, not the Composite pattern to resolve the test duplication. Did you mean composition in some other sense?

Cheers,
Ben

Posted by: Ben Johnston at January 31, 2005 11:00 PM

Ben: "composition" does not necessarily refer to the Composite pattern. The Composite pattern describes when and how you might use an "atomic" object and a composite object in the same way by making them both be instances of the same abstract type. "Composition" refers to the general act of composing objects together in any way.

Posted by: Nat Pryce at February 1, 2005 1:50 PM

Many thanks for the comments. I am using the term composition in a more general sense (e.g. google for "composition vs inheritance"):

http://www.cafeaulait.org/course/week4/14.html

I guess this equates to Ben's use of the term "delegation". (BTW - as a former Self programmer, I use the term "delegation" to mean something slightly different than that, but that's a whole other article, see http://ivan.truemesh.com/archives/000226.html).

To use Patternspeak, maybe I should have titled this "The 'replace template method with strategy pattern' refactoring"?

Posted by: ivan at February 2, 2005 6:48 PM

Nice article.

I used to use inheritance quite a lot years ago and almost always ended up replacing it with delegation in the end. Nowadays I generally always use delegation even for things that might seem more cleanly implemented with inheritance.

I don't think the human brain is good at understanding what impact making changes to inheritance hierarchies might have particularly when doing refactoring and most IDE's do not support it well.

For example if you change a method signature in a subclass which was overriding a method in the parent the IDE should really flash up a warning just incase you weren't aware that what appeared to be a method rename actually resulted in one new method and one inherited method with the same name as the original. It's particularly easy to miss as you don't get compile time breakages you get a behavioural change.

And as for inherited tests - ick!

Posted by: Toast Man at February 10, 2005 3:01 AM

Excellent article and comments. I second Toast Man's comment about the danger of method renames where inheritance is involved.

Are there any situations where inheritance is preferred? I haven't researched this much, but it seems odd that the very first OO concept I learned in college seems to be falling out of favor.

Posted by: Bryan Klumpp at June 25, 2005 11:16 PM