views:

125

answers:

6

I sometimes end up with a class hierarchy where I have an abstract base class with some common functionality and a couple of implementing classes that fall into two (rarely more) groups which I want to treat differently in some cases. An example would be an abstract tree node class and different branch and leaf implementations where I want to distinguish branches and leaves at some point.

These intermediate classes are then only used for "is-a" statements in flow control and they don't contain any code, although I have had cases where they "grew" some code later.

Does that seem smelly to you? In my tree example, one alternative would be to add isLeaf() / isBranch() abstract methods to the base class and implement those on the intermediate classes, but that didn't seem to be any better to me, really, unless I'd mean to have classes that could be multiple things at once.

+2  A: 

Yes, deep inheritance hierarchies are a code smell anyway.

parkr
A: 

A class that doesn't contain any code is definitely a code-smell....

Quagmire
+1  A: 

In general, empty classes are a code smell.

I agree your isLeaf or isBranch methods are a correct alternative. They add information about the objects , which is helpful. (This is because, on the super class, you can't express that subclasses are "either leaf or branch").


The two methods with opposite results might also be considered as code duplication.

You could use only one... But I would recommend return an enumerated value LEAF or BRANCH.

KLE
+4  A: 

To me, using "is-a" tests in flow control is just as smelly as using switch/case. In a good OO design, neither is needed.

Ber
I agree, this is the more critical problem.
Noon Silk
+1 I agree. Although this type of non-OO design could be used temporarily before refactoring, or could also be used if a different class should hold an aspect (separating responsibilities). **Finding the exact balance of class hierarchies is a difficult and evolutionnary process** usually.
KLE
I agree 100 percent. I can't tell you how many arguments I've been in about switch statements not being good OO.
Chuck Conway
+2  A: 

Yup, definitely a code smell -- don't code these empty classes unless you're ready to write that code into it. Think YAGNI (you aint gonna need it) -- don't do it unless you need it already.

Also, have you considered cases wherein these classes are only there to provide abstract methods, or to group them based on capabilities in terms of methods or properties?

If that's the case, maybe what you really need are interfaces, not additional abstract classes?

Jon Limjap
Thanks for the additional suggestions. The cases I'm talking about, interfaces wouldn't help me, though.
Hanno Fietz
A: 

Seems alright to me, if you're going to put new functionality in later.

But if not, an enum is generally used here.

-- Edit

Though I must agree with Ber, that you shouldn't generally be using 'is-a' anyway.

Noon Silk