Which design choice is better?

[kk_oop]

Hi. I just attended a design review where something funky was
presented. I recommended a different approach but was unable to
convince others that my approach was worth doing. I wanted to get some
feedback from this group on this problem.

We have three classes, A, B and C. Class A is abstract. B extends A
and provides implementation for all of A's abstract methods. Part of
this implementation includes the definition of three private methods in
B, let's call them do1, do2 and do3. These methods can be implemented
in two different ways based on some criteria. So the problem at hand
was how to handle the two different implementations.

The designer did this:

B extends A and adds private methods do1, do2 and do3.
C extends B and overrides do1, do2 and do3 with its own private
implementations. So some clients will use B polymorphicaly through A,
and other clients will use C polymorphically through A.

This works, but it seemed to couple parts of C to B unnecessarily.
Essentially, if I change do1, do2 or do3 implementation in B, C will be
impacted since it extends B. That doesn't seem like a good thing,
since C doesn't really care at all about B's implementation of these
methods.

I recommended using the template method pattern instead. Basically, I
said to leave do1, do2 and do3 abstract in B. Then make a C1 and a C2.
Each would extend B and provide its own implementation of do1, do2 and
do3.

The designer's response was that there would never be more than these
two variations of do1/do2/do3, so the template method pattern's
scalability benefit was a non-issue. Also, these three methods were
pretty simple, so decoupling the C and B implementations was not a big
factor. What was a big factor was adding the extra class (C1 and C2
instead of just C), and making the code a bit more complex from an OO
perspective--which would make it harder for folks new to OO to
understand (we have many folks on the project who are new to OO).

I can see his point, but it still just doesn't sit right.

Any thoughts for or against either approach?

Thanks for any input!

Ken

 

[Matt Humphrey]

Hi. I just attended a design review where something funky was
presented. I recommended a different approach but was unable to
convince others that my approach was worth doing. I wanted to get some
feedback from this group on this problem.

We have three classes, A, B and C. Class A is abstract. B extends A
and provides implementation for all of A's abstract methods. Part of
this implementation includes the definition of three private methods in
B, let's call them do1, do2 and do3. These methods can be implemented
in two different ways based on some criteria. So the problem at hand
was how to handle the two different implementations.

The designer did this:

B extends A and adds private methods do1, do2 and do3.
C extends B and overrides do1, do2 and do3 with its own private
implementations. So some clients will use B polymorphicaly through A,
and other clients will use C polymorphically through A.

Ah, a trick question. Private methods cannot be overridden. Do you mean
that B and C really have no relation through do1, do2 & do3? Or do you mean
private as in part of the implementation, but scoped for subclasses, as in
protected, etc?

This works, but it seemed to couple parts of C to B unnecessarily.
Essentially, if I change do1, do2 or do3 implementation in B, C will be
impacted since it extends B. That doesn't seem like a good thing,
since C doesn't really care at all about B's implementation of these
methods.

If they're truly private (as you first said) there is no coupling between B
and C, except any other protected / public parts of B that C might use
normally.

I recommended using the template method pattern instead. Basically, I
said to leave do1, do2 and do3 abstract in B. Then make a C1 and a C2.
Each would extend B and provide its own implementation of do1, do2 and
do3.

This only makes sense if the API proposed by do1,do2 and do3 is truly more
generic than simply B's implementation, which seems somewhat unlikely.

The designer's response was that there would never be more than these
two variations of do1/do2/do3, so the template method pattern's
scalability benefit was a non-issue. Also, these three methods were
pretty simple, so decoupling the C and B implementations was not a big
factor. What was a big factor was adding the extra class (C1 and C2
instead of just C), and making the code a bit more complex from an OO
perspective--which would make it harder for folks new to OO to
understand (we have many folks on the project who are new to OO).

Having overdesigned classes myself with hierarchies that are too deep, I can
appreciate this point. If the two classes really only differ by their
implementations (A really is defining the protocol) their details should not
be interwoven.

I can see his point, but it still just doesn't sit right.

"Unnecessary consistency is the hobgoblin of small minds."

Any thoughts for or against either approach?

It really hangs on whether do1,do2,do3 are really private, in which case
this is a non-issue. If you can make a case that there may reasonably be
additional subclasses AND that these subclasses will still conform to the
do1,do2,do3 protocol, the template pattern would be a better choice.

Cheers,
Matt Humphrey (e-mail address removed) http://www.iviz.com/

 

[Wibble]

Hi. I just attended a design review where something funky was
presented. I recommended a different approach but was unable to
convince others that my approach was worth doing. I wanted to get some
feedback from this group on this problem.

We have three classes, A, B and C. Class A is abstract. B extends A
and provides implementation for all of A's abstract methods. Part of
this implementation includes the definition of three private methods in
B, let's call them do1, do2 and do3. These methods can be implemented
in two different ways based on some criteria. So the problem at hand
was how to handle the two different implementations.

The designer did this:

B extends A and adds private methods do1, do2 and do3.
C extends B and overrides do1, do2 and do3 with its own private
implementations. So some clients will use B polymorphicaly through A,
and other clients will use C polymorphically through A.

This works, but it seemed to couple parts of C to B unnecessarily.
Essentially, if I change do1, do2 or do3 implementation in B, C will be
impacted since it extends B. That doesn't seem like a good thing,
since C doesn't really care at all about B's implementation of these
methods.

I recommended using the template method pattern instead. Basically, I
said to leave do1, do2 and do3 abstract in B. Then make a C1 and a C2.
Each would extend B and provide its own implementation of do1, do2 and
do3.

The designer's response was that there would never be more than these
two variations of do1/do2/do3, so the template method pattern's
scalability benefit was a non-issue. Also, these three methods were
pretty simple, so decoupling the C and B implementations was not a big
factor. What was a big factor was adding the extra class (C1 and C2
instead of just C), and making the code a bit more complex from an OO
perspective--which would make it harder for folks new to OO to
understand (we have many folks on the project who are new to OO).

I can see his point, but it still just doesn't sit right.

Any thoughts for or against either approach?

Thanks for any input!

Ken

Generally, the hardest questions to answer are the ones where
the result makes the least difference. If theres only two of
these things, how much time is worth wasting on this, or is your
time very cheap?

 

[Thomas Weidenfeller]

The designer did this:

B extends A and adds private methods do1, do2 and do3.
C extends B and overrides do1, do2 and do3 with its own private
implementations. So some clients will use B polymorphicaly through A,
and other clients will use C polymorphically through A. [...]
I recommended using the template method pattern instead. Basically, I
said to leave do1, do2 and do3 abstract in B. Then make a C1 and a C2.
Each would extend B and provide its own implementation of do1, do2 and
do3.

Does it matter? More precisely, assuming that A, B, C are some
real-world abstractions, or are they just some artificial "helper"
classes? If they are abstractions of the real world, I would use the
architecture which as close as possible mirrors the real word. Not
because that architecture might be better, but just to avoid confusion
and have a better mapping of the problem domain to the solution domain.

If they not closely resemble some real world artifact, then which
programming religion do you follow? If you are, for example, into Agile
Methods, like XP, then apply the rules of these method (in XP you will
find "Do the simplest thing that could possibly work", and "You ain't
gone need it"). You see where your programmer's argument against the
template pattern comes from?

/Thomas

 

[Dale King]

Hi. I just attended a design review where something funky was
presented. I recommended a different approach but was unable to
convince others that my approach was worth doing. I wanted to get some
feedback from this group on this problem.

We have three classes, A, B and C. Class A is abstract. B extends A
and provides implementation for all of A's abstract methods. Part of
this implementation includes the definition of three private methods in
B, let's call them do1, do2 and do3. These methods can be implemented
in two different ways based on some criteria. So the problem at hand
was how to handle the two different implementations.

The designer did this:

B extends A and adds private methods do1, do2 and do3.
C extends B and overrides do1, do2 and do3 with its own private
implementations. So some clients will use B polymorphicaly through A,
and other clients will use C polymorphically through A.

This works, but it seemed to couple parts of C to B unnecessarily.
Essentially, if I change do1, do2 or do3 implementation in B, C will be
impacted since it extends B. That doesn't seem like a good thing,
since C doesn't really care at all about B's implementation of these
methods.

I recommended using the template method pattern instead. Basically, I
said to leave do1, do2 and do3 abstract in B. Then make a C1 and a C2.
Each would extend B and provide its own implementation of do1, do2 and
do3.

The question then would be are there other classes than B that extend
abstract class A. If not, then A and B in your scenario could be
combined into one class and that would probably be more acceptable to
your coworkers.

There are some design principles that some use that say that you should
never subclass any class that is not abstract. I'm not sure I agree with
it being absolute, but in general it is a good principle. I think the
same goes for methods themselves. I think if you see a method that
overrides a method that already has an implementation that /might/ be an
indication of a code smell (to use the term coined by Martin Fowler).

I think I agree with you that your design is better, but agree with
others that perhaps it is not worth losing too much sleep over.

It reminds me of the saying that "Any problem in computer science can be
solved with another layer of indirection".

 

[John C. Bollinger]

Ah, a trick question. Private methods cannot be overridden. Do you mean
that B and C really have no relation through do1, do2 & do3? Or do you mean
private as in part of the implementation, but scoped for subclasses, as in
protected, etc?

This is a KEY question. To underscore and expand on what Matt already
wrote, methods declared "private" are not virtual (which is just a
different way of saying that they cannot be overridden). References to
those methods from B's other method's will ALWAYS use B's versions.
*This applies even to B's methods inherited by subclass C.* (Yet
another way of saying that private private methods cannot be overridden.)

If they're truly private (as you first said) there is no coupling between B
and C, except any other protected / public parts of B that C might use
normally.

Moreover, changing methods of B that are overridden by C can only affect
C if C invokes those methods directly. If it does so then it is in any
case relying on B's implementation, *whatever that might be*, and the
change in C's behavior in such a case would be expected and desired.

This only makes sense if the API proposed by do1,do2 and do3 is truly more
generic than simply B's implementation, which seems somewhat unlikely.

I agree. An alternative worth consideration is to use composition
instead of inheritance to solve this problem; since we've entered into
design pattern space, that could be described as applying the Strategy
pattern.

Is he right? Is there indeed no possibility of any other alternatives
being needed in the future?

I don't see coupling being an issue in any case, particularly if all C
attempts to do is to override the three methods of B that are under
discussion.

So, the argument is that because many folks on the project are new to
OO, you should not adhere to good OO principles? It seems to me that
that's asking for trouble. If you are turning these people loose on an
OO language then it seems to me that you should be doing as much as
possible to bring them up to speed. Abandoning OO principles is not the
way to accomplish that. Nevertheless, this is a strategy decision that
I cannot make for your outfit. The payoffs from good OO are often
medium- to long-term, and they often don't stand out on their own.
Getting productivity out of your personnel, on the other hand, has
measurable short-term benefits.

Having overdesigned classes myself with hierarchies that are too deep, I can
appreciate this point. If the two classes really only differ by their
implementations (A really is defining the protocol) their details should not
be interwoven.

And here I revisit my point about considering design by composition. It
is an excellent means to simplify complicated inheritance hierarchies
where bits and pieces of implementation are suitable for reuse in
various places. If, as a matter of design, B and C differ only in three
methods out of ten (say), then the other seven should come from some
common source.