While experimenting for my previous Sparse Array post I wanted to be able to easily change between using a HashMap and a Sparse Array approach in my code so I would be able to quickly make measurements with both. To achieve this at the time I thoughtlessly came up with an inheritance approach. At the time I thought that it would be better to be refactored to composition but because I liked the idea to do the refactoring and describing it in a post I decided to leave it for later. And this later time has come! 🙂

Inheritance vs Composition

You might also have read that we should “prefer Composition over Inheritance”. If you look closely you will notice that this rule of thumb implies that we should only use Inheritance only on certain conditions.

The incentive

I believe that the mantra stems from the fact that in many cases we blindly tend to go to inheritance to solve a common problem: What should we do with code that is the same on more than one classes? Why not just create a  base parent class and put the common code there? There is also the ‘extract superclass’ refactoring which makes this a breeze. I must confess I have fallen in this trap very often and I actually did it for the tree nodes where  I copy-pasted the two Node implementing classes, saw my isEnd find and getter-setter methods were duplicated and did an ‘extract superclass’ refactoring and my code duplication problem was solved.

The downside

OK, problem solved, why should we bother with it? What is the problem with it? Based on my experiences after a while we might get into the following situation:

This code we reused, is providing some pretty useful functionality and we would like to also reuse it in another class. But this class already inherits from a different class! And in Java we can only inherit from one class. In most non-trivial real world code any attempt to reuse this code will become a major refactoring effort.  Likely this code will have been living along with other code with which it will have become intertwined. This will blur the boundaries of what we need to refactore. In turn this would lead to a lot of effort to study, identify the exact code we need and then pry it from the rest of the code.

The exchange

The above mentioned prerequisite for refactoring can lead to the following discussion between users, managers, BAs, POs(I will call them Non-devs from now on) and developers:

-Non-Devs: The new XYZ behavior we have in the A component code has proven  really useful to our customers and we have created issue 123456 to also add this behavior to B component that they use directly.

-Devs: Ok sounds good. Let us take a look at it.

(some time passes)

-Devs: Hey guys, we have started working on issue 123456, but things are not going well. It has proven difficult to reuse the code we had in component A for component B. We are going to have to do a lot of refactoring work on component A. This will also entail a lot of risk for breaking A so we we have to be really careful and vigilant with our testing.
-Non-Devs: But we have already done this work! Why would this take all this time? This is just a small functional change. This does not make sense to us!
-Devs: So what should we do? Do you want us to proceed or not with this?
-Non-Devs: Just leave it for now. We will look into it at some point in the future.

( A few weeks pass. And we get closer to the release)

-Non-Devs: Issue 123456 has been escalated by our customers and it is critical that we include it for our next release.

-Devs: Oh!! But we discussed about this and it is not small work[Devs thinking: *beep*… will have to again skip time with kids, time with spouse, gym, going out etc]
OK… We get on with it

The example

The inheritance approach

Currently we have the following classes:

inheritance

Following is the interface that describes our Node. For a node we can store whether this node represents the end of a word and also the other nodes that are connected to this one representing next letters for words.

public interface Node {
  Node getNextNode(Character nextNodeChar);
  void putNextNode(Character nextNodeChar, Node refNode);
  void setIsEnd(boolean isEnd);
  boolean isEnd();
}

And the abstract class that our two specialization inherit from. This class expresses my desire to reuse code since it holds the field and functions that were the same for all Node classes.

abstract class AbstractNode implements Node {
    private boolean isEnd = false;

    @Override
    public void setIsEnd(boolean isEnd) {
        this.isEnd = isEnd;
    }

    @Override
    public boolean isEnd() {
        return isEnd;
    }
}

This is the implementation that uses a HashMap. In the very first version of this class this class also contained the isEnd field and getter and setter methods.

class HashMapNode extends AbstractNode {

    private final Map<Character, Node> nodeMap
                                = new HashMap<>();

    @Override
    public Node getNextNode(Character nextNodeChar) {
        return nodeMap.get(nextNodeChar);
    }

    @Override
    public void putNextNode(Character nextNodeChar,
                            Node refNode) {

        nodeMap.put(nextNodeChar, refNode);
    }
}

And this is the class implementing the Sparse Array. This class also had in its first version a isEnd field and its getter and setter. So this led me to identify these three elements as code duplication and led me to extracting the Abstract supperclass to remove it.

class GreekUppercaseSparseNode extends AbstractNode {
    private static final int ARRAY_SIZE = 25;

    private final Node[] sparseArray
                           = new Node[ARRAY_SIZE];

    @Override
    public Node getNextNode(Character nextNodeChar) {
        return sparseArray[nextNodeChar - 0x0391];
    }

    @Override
    public void putNextNode(Character nextNodeChar,
                            Node refNode) {
        sparseArray[nextNodeChar - 0x0391] = refNode;
    }
}

OK this is the current state where we have one interface, one abstract class and two concrete classes and we want to remove inheritance and use composition. How do we start? What helps me most is the following quote:

… what else can we do? … encapsulate the concept that varies.

–GoF, Design Patterns (Chapter 2)

When looking at this advice a couple of interesting questions start to pop up. What is it that varies? What is the concept? How to encapsulate?   I like to tackle these in the reverse order to how they appear in the quote.

So let’s ask the questions for our current case:

What is it that varies?  In this case it is the implementation of the getNextNode() and putNextNode() methods.

What is the concept? It is how we map Characters to Nodes

This leads to say that we need a mechanism that provides functionality to map Characters to Nodes that will have the getNextNode() and putNextNode() methods.

With the above in mind I answer the question ‘How to encapsulate?’  as follows. I will encapsulate by creating CharacterToNodeMapper interface and two concrete implementations one for a HashMap and the other for SparseArray.

Here is how the new interface would look like:

public interface CharacterToNodeMapper {
    Node getNextNode(Character nextNodeChar);
    void putNextNode(Character nextNodeChar, Node refNode);
}

If we look closely to our previous concrete classes they only implement the of the CharacterToNodeMapper interface so I will simply change their names, remove the extends AbstractNode clause and add the implements CharacterToNodeMapper clause.

So they come to the following form:

class HashMapMapper
                implements CharacterToNodeMapper {

    private final Map<Character, Node> nodeMap
                                   = new HashMap<>();

    @Override
    public Node getNextNode(Character nextNodeChar) {
        return nodeMap.get(nextNodeChar);
    }

    @Override
    public void putNextNode(Character nextNodeChar,
                            Node refNode) {

        nodeMap.put(nextNodeChar, refNode);
    }
}
class GreekUppercaseSparseMapper
                implements CharacterToNodeMapper {

    private static final int ARRAY_SIZE = 25;

    private final Node[] sparseArray
                           = new Node[ARRAY_SIZE];

    @Override
    public Node getNextNode(Character nextNodeChar) {
        return sparseArray[nextNodeChar - 0x0391];
    }

    @Override
    public void putNextNode(Character nextNodeChar,
                            Node refNode) {
        sparseArray[nextNodeChar - 0x0391] = refNode;
    }
}

No we are left with AbstractNode that has no child classes and here is the area that we want to apply the composition. So I am going to remove the abstract keyword and rename it

class NodeImpl implements Node {

I will also add a field for the composition object and a constructor for the factory to set it up.

    private CharacterToNodeMapper mapper;

    NodeImpl(CharacterToNodeMapper mapper) {
        this.mapper = mapper;
    }

Since the class is no longer abstract the getNextNode() and putNextNode() methods of the Node interface must be implemented. And because now the actual implementing code is behind the mapper instance we also need to delegate the calls to it.

    @Override
    public Node getNextNode(Character nextNodeChar) {
        return mapper.getNextNode(nextNodeChar);
    }

    @Override
    public void putNextNode(Character nextNodeChar,
                            Node refNode) {
        mapper.putNextNode(nextNodeChar, refNode);
    }

The final class that we get after all the changes in AbstractNode is the following

class NodeImpl implements Node {

    private boolean isEnd = false;

    private CharacterToNodeMapper mapper;

    NodeImpl(CharacterToNodeMapper mapper) {
        this.mapper = mapper;
    }

    @Override
    public Node getNextNode(Character nextNodeChar) {
        return mapper.getNextNode(nextNodeChar);
    }

    @Override
    public void putNextNode(Character nextNodeChar,
                            Node refNode) {
        mapper.putNextNode(nextNodeChar, refNode);
    }

    @Override
    public void setIsEnd(boolean isEnd) {
        this.isEnd = isEnd;
    }

    @Override
    public boolean isEnd() {
        return isEnd;
    }
}

The final diagram from the above classes is now the following:

composition

Also in the factory the actual note instantiation, for example for the HashMap case, is changed from:

return new HashMapNode();

to:

return new NodeImpl(new HashMapMapper());

Code

You can find the resulting code for this post in the following release on GitHub

Conclusion

Inheritance is a fundamental part of OO languages. But I have and have seen it a lot to be used as the go-to mechanism to achieve code reuse and minimize duplication in applications. This would lead the codebase to become more rigid and difficult to change.

We should think twice when applying inheritance and definitely avoid doing it when our goal is code reuse. My rule of thumb, which I have learned from Patroklos Papapetrou, is to only apply inheritance when I am trying to express the semantic relationships of classes. Hopefully my step by step thought process I presented here will help when trying to reduce code duplication with composition.
And remember… when using single inheritance languages, we have only one[AKA single] chance to inherit, so only one chance to get it right 🙂

Thanks for reading!

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s