is it wise to provide hooks to customize ExceptionalUnitGraph's constructors?

[John Jorgensen <jorgnsn@lcd.uregina.ca>]

About two weeks ago, I suggested to Venkatesh Ranganath
(whose code I broke by reorganizing Soot's CFG classes) that he
could extend ExceptionalUnitGraph instead of UnitGraph, and that
he might be able to get the graphs he wants by selectively
overriding methods called by the ExceptionalUnitGraph
constructor.

A week ago, Venkatesh told me about a problem with my
suggestion to override ExceptionalUnitGraph and suggested a
solution. The solution cleanly solves the immediate difficulty,
but it made me realize that by encouraging people to extend
Soot's concrete graph classes---especially when the subclass
needs to change what is produced by the superclass's
constructor---I may be unwisely limiting the scope for future
changes to the graph implementations.  So I'm looking for advice
on how to proceed, while the graph changes are still
unreleased and Venkatesh is still the only person who will suffer
from my waffling.

First, the hitch that Venkatesh encountered while trying to
extend ExceptionalUnitGraph:

The ExceptionalUnitGraph constructor has this basic form:

  public ExceptionalUnitGraph(Body b, ThrowAnalysis t, boolean o) {

    super(b);

    buildUnexceptionalEdges(); // Establish regular control flow.

    buildExceptionDests();     // Determine which units throw
			       // which exceptions to which
			       // handlers.

    buildExceptionalEdges();   // Add CFG edges that correspond
			       // to the caught exceptions.

    buildHeadsAndTails();
  }

buildUnexceptionalEdges() is inherited from UnitGraph while the
other methods are defined in ExceptionalUnitGraph. All are
"protected".  

My understanding is that Venkatesh wants to filter out some graph
edges, depending on input from a user.  I suggested that if
Venkatesh couldn't get the set of graph edges he wants by passing
a customized ThrowAnalysis, he could override one or both of
buildExceptionDests() or buildExceptionalEdges().  He replied
that overriding buildExceptionalEdges() could do what he wants
except for one hitch: his buildExceptionalEdges() would need
information provided as a parameter to his subclass's
constructor, but since the first thing a subclass constructor
does is to invoke its superclass's constructor, there would be no
opportunity to do anything with the subclass parameters until
after the superclass constructor had already called
buildExceptionalEdges().

Venkatesh suggested moving the bulk of the current constructor
into a separate method.  The public ExceptionalUnitGraph
constructor would call this method to actually build the graph,
while a new protected constructor would not, like this:

  public ExceptionalUnitGraph(Body b, ThrowAnalysis t, boolean o) {
    super(b);
    initialize(t, o);
  }

  protected ExceptionalUnitGraph(Body b) {
    super(b);
  }

  protected void initialize(ThrowAnalysis t, boolean o) {
    buildUnexceptionalEdges();
    buildExceptionDests();
    buildExceptionalEdges();
    buildHeadsAndTails();
  }

Subclass constructors could then delay calling initialize() until
after they perform subclass-specific initialization.

This is a clever way of dealing with the problem at hand, but it
gives the subclass the responsibility of ensuring that the
superclass is initialized properly. That makes me reconsider the
wisdom of encouraging the subclassing of ExceptionalUnitGraph.  

(Incidentally, I can imagine Rube Goldbergian schemes that would
allow Venkatesh to do what he wanted without modifying
ExceptionalUnitGraph: things like providing his subclass with a
static method, stashMyParameters(...), that tucked the
information required by his buildExceptionalEdges() into some
static variables from which his it could retrieve them.  Not only
would that be an ugly kludge, though, it would still depend on
the internal organization of ExceptionalUnitGraph's constructor,
and it's the wisdom of that commitment that I'm starting to
doubt.)

I think I can explain my misgivings in terms of the 
different ways to think about inheritance.  

My reorganization of the graph classes focuses on the inheritance
of implementation, to minimize duplicated code while keeping
subclass-specific code in the subclasses. Venkatesh's suggestion
widens the scope for inheriting implementation by providing
another hook to customize the inherited code.

Another aspect of inheritance is the inheritance of an
interface.  Now in a narrow sense, Java's language definition
guarantees that when you define class C2 by extending C1, C2 will
have the same interface:  all the public and protected methods of
C1 will be present in C2, with the same signatures and return
types.

But there's a fuzzier notion of what it means to inherit an
interface: calling C2.m() should in some sense mean the same
thing as calling C1.m().  I presume that maintaining this
conceptual interface is what textbooks have in mind when they
encourage us to think of "C2 extends C1" as meaning "C2 is a C1".

To provide a whimsical illustration, my initial misgivings were
that I might have created a sensible hierarchy akin to

   Dog is-a Mammal is-a Vertebrate is-a Creature

and then, by encouraging them to reuse code in Dog, I was
tempting programmers into using Dog to implement Cat. That might
well save code, but callers of bark() don't expect to hear
"meow".

These misgivings about the appropriate use of subclassing are not
by themselves a reason to bar extension to ExceptionalUnitGraph:
Soot is a framework for programmers to build on; they have to
decide for themselves which extensions make sense and which
tradeoffs between expedience and clarity are worthwhile.  But the
conceptual misgivings lead me to a more pragmatic concern.

By encouraging people to subclass the graph classes, I am
implicitly guaranteeing client code that the ExceptionalUnitGraph
constructor will always be structured the way it is now.  But I'm
not smart enough to think of all the ways that
ExceptionalUnitGraph might need to change in the future, nor to
re-re-organize it so that the code appropriate for reuse is
factored into abstract classes (i.e., I can see that Dog.bark()
should be replaced with a parameterized Creature.speak(), but
it's not so clear to me how to factor out the reusable parts of
ExceptionalUnitGraph.)

While thinking about this I reread Items 14, "Favor Composition
over Inheritance", and 15, "Design and Document for Inheritance
or else Prohibit it", in Joshua Bloch's "Effective Java". In
light of the former, I've asked Venkatesh if he can get what he
needs by creating an instance of ExceptionalUnitGraph within a
wrapper class. In light of the latter, I'm leaning to changing
most of the protected hooks I've introduced into some
soot.toolkits.graph constructors into package private members;
that would let us continue to inherit implementations within the
package without turning the implementation into part of the
interface to which we are committed.

Any insights?