Is it code-smelly to have empty classes in the middle of a class hierarchy?

Question

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.

Answer 1

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.

Answer 2

Yes, deep inheritance hierarchies are a code smell anyway.

Answer 3

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?

Answer 4

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.