My "anti-patterns" - 2

Continuing the "anti-patterns" subject. By the way, "anti-patterns" are not, strictly speaking, the "anti" of "patterns". "Anti-patterns" are specifically "don't do" while patterns do not necessarily mean "do". However, it's much easier to say "don't" than to say "do". Out of the 10 commandments the vast majority is "don't". Out of 613 mitzvot, 365 are negative. So let's continue the wave...

Inheritance

I just love to make controversial statements! So bombastic - do not use inheritance! OK, you know it was a joke. But! I see so much cases of inheritance being used for no good reason that I'm becoming worried. It seems that the new generation of developers was raised on a hype of object-oriented. Functions are passé, we should program in objects! The next (highly dangerous!) clause coming out of this is "when there's a common piece of code, one should use inheritance". Wrong, wrong, completely wrong!

Several horrible constructions come out of this thinking. Abstract classes that are full of unrelated methods placed there only in order to be accessible in subclasses. "This new class is almost like the one we have - let's derive it from the latter." "We have a flow that differs in some details - let's create a super-class with a couple of abstract methods and implement them in subclasses. The latter is called "template pattern". Finally, there's a pure abuse of inheritance (especially interfaces) that has to do with creating small abstract classes (or interfaces) no one really uses.

Let's look at these examples one by one. In this case there is already an hierarchy of classes (for whatever reason) and two of them need to share some code (generally a utility function). The temptation is to put this code into one of the abstract classes common to both. I guess it comes from the junior developers' fear to create new units of code - be it classes, interfaces or whole modules. It turns out this requires some sort of courage (who would have thought!). So a new protected method gets created in an abstract class.

Two immediate problems come out of this - the fact that the method is placed in the abstract class invites future users of this method to disguise themselves as members of this hierarchy of classes (otherwise they won't get access to this useful method) without real reason. The second problem arises when one tries to refactor the abstract class to a new infrastructure module. This offensive method in many cases knows some abstractions of higher levels and prevents the abstract class from being extracted.

It's generally very easy to find out these "weeds" - try to put the word 'static' before the definition of such method. If the code continues to compile, you can safely extract it out to a utility class (yes, there's nothing wrong in using utility classes with static methods!).

Second case - perhaps the most dangerous one. There's a class that does almost what we want? Let's extend it and override the offending method(s). This specifically creates inheritance for no good reason. In this case the most important thing to look for is a possible violation of the Liskov substitution principle. Can an instance of this new class be used in whatever context an instance of existing class is being used? Probably not. So try to refactor the existing class. Either make it suite your needs or take out the required functionality to another place. Beware of the Conway's Law.

The third case (aka template pattern) may be an evolution of the previous one but may also be created on its own. It's often characterized by an abstract class having methods 'postSomething' and 'preSomething'. Very curious idea - naming a method not according to what it should do but to when it is supposed to be called - sort of callbacks. However, contrary to callbacks which are supposed to react to events, there are no events here. For instance:

public abstract class AbstractCommand {
    public final void execute() {
        validate();
        doExecute();
        postProcess();
    }
    protected abstract void validate();
    protected abstract void doExecute();
    protected abstract void postProcess();
}

Wow! What a code! Excuse me, what exactly this "design" gives? To prevent any future command from forgetting to "validate"? And, as you may guess, 28 out of 30 commands have their 'postProcess' method empty. By the way, another LessAbstractCommand quickly comes up with a method 'doExecute' doing some "common" stuff and calling 'doRealExecute'. Within a couple of levels one quickly gets lost in those "real-real" executes. Great fun to debug as well!

Of course, this is an extreme example, but it's amazing how tempting it is to "establish a flow" and make derived classes only implement "plug" methods. Next time you are considering something of the king, think instead of an easy-to-use set of utilities, or corner stones to be used by future implementors. It's OK if more than one of them would call the same two methods in the same sequence - no harm done. This is not the kind of code that should be reused. With the template method, however intelligent you are, you won't be able to cover for all cases in your "universal" flow and one day your successor will add an ugly "prePostSomething" method implemented in only one subclass.

Finally! Little interfaces - one of my favorite ones. "Many of our objects have a name and a description - let's create a common interface (superclass) NamedEntity (AbstractNamedEntity)!" My favorite saying goes "the fact that both a kangaroo and a soldier are capable of running doesn't mean they should both implement Runnable". The main question you should ask yourself - will there be any code that gets an instance of any NamedEntity and treats it as such? Check yourself thoroughly and refrain from doing it.

I was once swearing hard when I had to study a system where every object that had a 'startup' and a 'shutdown' method was declared to implement a common interface. Surely there was no place in the system that handled all instances of this interface. But imagine what pain it was trying to understand where the 'startup' method of a certain object is called from. One gets 15 places, 14 of which are irrelevant for the specific case - now go find the right one!

So - is inheritance evil? No way! As indicated in a comment to my previous post - "guns don't kill, people do". Thanks, I liked it! But it's amazing how often the inheritance is being abused, this is all I want to say.