Code Smell: Inheritance Abuse

Question

It's been generally accepted in the OO community that one should "favor composition over inheritance". On the other hand, inheritance does provide both polymorphism and a straightforward, terse way of delegating everything to a base class unless explicitly overridden and is therefore extremely convenient and useful. Delegation can often (though not always) be verbose and brittle.

The most obvious and IMHO surest sign of inheritance abuse is violation of the Liskov Substitution Principle. What are some other signs that inheritance is The Wrong Tool for the Job even if it seems convenient?

Answer 1

When inheriting just to get functionality, you're probably abusing inheritance. This leads to the creation of a God Object.

Inheritance itself is not a problem as long as you see a real relation between the classes (like the classic examples, such as Dog extends Animal) and you're not putting methods on the parent class that doesn't make sense on some of it's children (for example, adding a Bark() method in the Animal class wouldn't make any sense in a Fish class that extends Animal).

If a class needs functionality, use composition (perhaps injecting the functionality provider into the constructor?). If a class needs TO BE like other, then use inheritance.

Answer 2

I would say, taking the risk of being shot down, that Inheritance is a code smell in itself :)

The issue with inheritance is that it can be used for two orthogonal purposes:

• interface (for polymorphism)
• implementation (for code reuse)

Having a single mechanism to get both is what leads in "inheritance abuse" in the first place, since most people expect inheritance to be about interface, but it might be used to get default implementation even by otherwise careful programmers (it's just so easy to overlook this...)

In fact, modern languages like Haskell or Go, have abandonned inheritance in order to separate both concerns.

Back on track:

A violation of the Liskov Principle is therefore the surest sign, since it means that the "interface" part of the contract is not respected.

However even when the interface is respected, you might have objects inheriting from "fat" base classes just because one of the methods was deemed useful.

Therefore the Liskov Principle in itself is not enough, what you need to know is whether or not the polymorphism is used. If it isn't, then there wasn't much point in inheriting in the first place, that's an abuse.

Summary:

• enforce the Liskov Principle
• check it's actually used

A way around it:

Imposing a clear separation of concerns:

• only inherit from interfaces
• use composition to delegate implementation

means that at least a couple keystrokes are needed for each method, and suddenly people begin to think about whether or not it's such a great idea to reuse this "fat" class for just a small piece of it.

Answer 3

Is it really "generally accepted"? It's an idea I've heard a few times, but it's hardly a universally-recognized principle. As you just pointed out, inheritance and polymorphism are the hallmarks of OOP. Without them, you've just got procedural programming with goofy syntax.

Composition is a tool for building objects a certain way. Inheritance is a tool for building objects a different way. There's nothing wrong with either of them; you just have to know how they both work and when it's appropriate to use each style.

Answer 4

The strength of inheritance is reusability. Of interface, data, behavior, collaborations. It's a useful pattern from which to convey attributes to new classes.

The drawback to using inheritance is reusability. The interface, data and behavior are encapsulated and transmitted en masse, making it an all-or-nothing proposition. The problems become especially evident as hierarchies become deeper, as properties which might be useful in the abstract become less so and begin to obscure properties at the local level.

Every class that uses the composition object will have to more-or-less re-implement the same code to interact with it. While this duplication requires some violation of DRY, it affords greater freedom for designers to pick-and-choose those properties of a class most meaningful to their domain. This is particularly advantageous when classes become more specialized.

In general, it should be preferred to create classes via composition, and then converting to inheritance those classes with the bulk of their properties in common, leaving the differences as compositions.

IOW, YAGNI (You Aren't Gonna Need Inheritance).

Answer 5

If you are subclassing A to class B but nobody cares if B inherits from A or not, so that's the sign that you might be doing it wrong.

Answer 6

I think battles like inheritance vs. composition, are really just aspects of a deeper core battle.

That fight is: what will result in the least amount of code, for the greatest expandability in future enhancements?

That's why the question is so hard to answer. In one language it could be that inheritance is more quickly put into place than composition than in some other language, or vice-versa.

Or it could be the specific problem you are solving in code benefits more from expedience than a long-term vision of growth. That's as much a business problem as a programming one.

So to get back to the question, inheritance could be wrong if it's putting you into a straight-jacket at some point in the future - or if it's creating code that does not need to be there (classes inheriting from others for no reason, or worse yet base classes that start to have to do a lot of conditional tests for children).

Answer 7

"Favor composition over inheritance" is the silliest of assertions. They are both essential elements of OOP. It's like saying "Favor hammers over saws".

Of course inheritance can be abused, any language feature can be abused, conditional statements can be abused.

Answer 8

Perhaps most obvious is when the inheriting class ends up with a pile of methods/properties that are never used in the exposed interface. In the worst cases, where the base class has abstract members, you'll find empty (or meaningless) implementations of the abstract members just to 'fill in the blanks' as it were.

More subtly, sometimes you see where in an effort to increase code reuse two things which have similar implementations have been squeezed into one implementation - despite those two things not being related. This tends to increase code coupling and disentangling them when it turns out that the similarity was just a coincidence can be quite painful unless it's spotted early.

Updated with a couple of examples: Although not directly to do with programming as such, I think the best example for this sort of thing is with localisation/internationalisation. In English there's one word for "yes". In other languages can be many, many more in order to agree grammatically with the question. Someone could say, ah let's have a string for "yes" and that's fine for lots of languages. Then they realise that the string "yes" doesn't really correspond to "yes" but actually the concept of "the affirmative answer to this question" - the fact that it's "yes" all the time is a coincidence and the time saved only having one "yes" is completely lost in disentangling the resulting mess.

I've seen one particularly nasty example of this in our code base where what was actually a parent -> child relationship where the parent and child had properties with similar names was modelled with inheritance. No-one noticed this as everything seemed to be working, then the behaviour of the child (actually base) and parent (subclass) needed to go in different directions. As luck would have it, this happened at exactly the wrong time when a deadline was looming - in attempting to tweak the model here and there the complexity (both in terms of logic and code duplication) went through the roof immediately. It was also really difficult to reason about/work with as the code simply didn't relate to how the business thought or talked about the concepts these classes represented. In the end we had to bite the bullet and do some major refactoring which could have been completely avoided if the original guy hadn't decided that a couple of similar sounding properties with values that happened to be the same represented the same concept.

Answer 9

There are situations when inheritance should be favored over composition, and the distinction is much more clear cut than a matter of style. I'm paraphrasing from Sutter and Alexandrescu's C++ Coding Standards here as my copy is on my bookshelf at work at the moment. Highly recommended reading, by the way.

First of all, inheritance is discouraged when unnecessary because it creates a very intimate, tightly coupled relationship between base and derived classes that can cause headaches down the road for maintenance. It's not just some guy's opinion that the syntax for inheritance makes it harder to read or something.

That being said, inheritance is encouraged when you want the derived class itself to be reused in contexts where the base class is currently being used, without having to modify the code in that context. For example, if you have code that is starting to look like if (type1) type1.foo(); else if (type2) type2.foo();, you probably have a very good case for looking at inheritance.

In summary, if you just want to reuse base class methods, use composition. If you want to use a derived class in code that currently uses a base class, use inheritance.