Re: Composition vs. Inheritance

On Monday 26 February 2007 08:35, Robbert Haarman wrote:
> I am intrigued by the suggestion that composition is somehow better
> than inheritance

It's not better.  Gamma's point is that people tend to view
inheritance as the be-all and end-all of OOP to the significant
exclusion of composition even when the latter is more appropriate.

I apologize for sounding like a broken LP record, but Gamma's
observation about the "tight coupling between the base class and the
subclass" is best illustrated by Joshua Bloch's example:

  http://courses.dce.harvard.edu/~cscie160/EffectiveJava.htm#lec012
  http://people.csail.mit.edu/gregs/ll1-discuss-archive-html/msg03912.html

Say, we want to implement a Set that counts the number of attempted
element insertions:

 | import java.util.HashSet;
 | import java.util.Collection;
 |
 | public class InstrumentedSet<E> extends HashSet<E> {
 |     private int addCount = 0; // number of attempted element insertions
 |
 |     public InstrumentedSet() {}
 |
 |     public InstrumentedSet(Collection<? extends E> c) {
 |         super(c);
 |     }
 |
 |     public boolean add(E o) {
 |         addCount++;
 |         return super.add(o);
 |     }
 |
 |     public boolean addAll(Collection<? extends E> c) {
 |         addCount += c.size();
 |         return super.addAll(c);
 |     }
 |
 |     public int getAddCount() {
 |         return addCount;
 |     }
 |
 | }

Pretty straightforward.

The problem with this class is that it produces the wrong count under
Sun's JDK.  Consider:

 | import java.util.Arrays;
 |
 | public class Main {
 |     public static void main(String[] args) {
 |         InstrumentedSet<String> set = new InstrumentedSet<String>();
 |         set.addAll(Arrays.asList(new String[] {"foo", "bar", "baz"}));
 |         System.out.printf("Number of attempted insertions: %d%n",
 |                           set.getAddCount());
 |     }
 | }

This prints:

 | $ java -showversion Main
 | java version "1.6.0"
 | Java(TM) SE Runtime Environment (build 1.6.0-b105)
 | Java HotSpot(TM) 64-Bit Server VM (build 1.6.0-b105, mixed mode)
 |
 | Number of attempted insertions: 6

The reason for this is because the #addAll method is implemented like
so in the superclass:

    | public boolean addAll(Collection<? extends E> c) {
    |     boolean changed = false;
    |     for (E elem : c) {
    |         if (this.add(elem)) { changed = true; }
    |     }
    |     return changed;
    | }

In other words, #addAll is implemented in terms of the overridable
method #add.  Since we've overridden #add in InstrumentedSet, the
overriden implementation of #addAll ends up counting each element
twice.

More abstractly, the problem is that callbacks are evil.

Suppose you have components X and Y such that X's function foo()
relies on Y's function bar().  Unbeknownst to X, Y's implementation of
bar() relies on X's function baz().  In other words, suppose we have:

 | X:
 |
 |   function foo() {
 |       temporarilyViolateInvariantsOfComponentX();
 |       result = Y.bar();
 |       restoreInvariantsOfComponentX(result);
 |   }
 |
 |   // Precondition: X's invariants hold at entry into baz()
 |   function baz() {
 |       doStuffNaivelyExpectingInvariantsToHold();
 |   }
 |
 | Y:
 |   function (bar) {
 |       partialResult = X.baz();
 |       return finishComputation(partialResult);
 |   }


Y calls back into X at a time when X has its invariants temporarily
messed up.  Hosage ensues.

Inheritance is just a particular example of the evilness of callbacks.

Possible solutions:

 1. Use composition if possible.  In the case of InstrumentedSet,
    composition solves the problem nicely, if somewhat verbosely in
    Java.  (In OOP languages that provide the moral equivalent of
    Smalltalk's doesNotUnderstand message, delegation is much terser.)

 2. Tightly control the extensibility points.  Basic pattern:

    | public abstract class WithResource {
    |     private final ResourceManager manager;
    |
    |     protected WithResource(ResourceManager manager) {
    |         if (manager == null) { throw new NullPointerException("manager"); }
    |         this.manager = manager;
    |     }
    |
    |     public final void run() {
    |         Resource resource = manager.acquireResource();
    |         try {
    |             doWork();
    |         } finally {
    |             manager.release(resource);
    |         }
    |     }
    |
    |     protected abstract doWork(Resource resource);
    | }

    Intended usage:

       | new WithResource(resourceManager) {
       |     public void doWork(Resource resource) {
       |         // do work with 'resource'
       |     }
       | }.run();

    No nasty surprises here, since you know exactly which methods will
    be overridden and which won't.

 3. Avoid self-use.

    Don't implement public overridable methods in terms of other
    public overridable methods in the same class.  In the above Set
    example, the superclass could (should?) have been implemented like
    so:

    | public boolean addAll(Collection<? extends E> c) {
    |     boolean changed = false;
    |     for (E elem : c) {
    |         if (this.__add(elem)) { changed = true; }
    |     }
    |     return changed;
    | }
    |
    | public boolean add(E elem) {
    |     return this.__add(elem);
    | }
    |
    | // not overridable
    | private boolean __add(E elem) {
    |     // add the element, if not already present
    |     // if already present, return false.  otherwise, true
    | }


Of these three options, Option #3 smacks of violated encapsulation.
For your class to be safely subclassable, you have to inform your
potential subclassers whether or not there is any self-use going on in
your current superclass implementation.  This reveals an
implementation detail that should, in theory, be hidden.

None of this is specific to Java (other than verboseness).