Why You Should Avoid Over-Abstracting

Back-end

Why You Should Avoid Over-Abstracting

Hannes Van De Vreken

Some time ago I started working on an existing project, so I read the documentation before diving in. At the top of the
contributing.md file there was this sentence: “Abstract when possible”. Quickly I learned the project contained more abstract classes than a normal project. This leads to too highly coupled, and often unchangeable code.

This post is dedicated on explaining why “abstract when possible” isn’t good advice. Not only in PHP, but in all programming languages.

First let me explain what is wrong with abstract classes. After that I will show you when it is OK to create them.

What is wrong usage of abstract classes?

Let me show you a couple of code samples.

Overcomplicated abstracted methods

First is a base class with an overcomplicated method.

Clearly what’s happening here is: the base class had a get method which returns a list of models. As more use cases appear, the base class doesn’t have enough flexibility. The get method is being changed to take an optional argument $options. As the amount of possible use cases grows, the get method’s LOC (lines of code) grows as well.

In the end the get method is chock-full of bugs, too hard to test and too hard to adapt without breaking it. Developers that need slightly different behavior for their AbstractRepository implementation will eventually override the get method with their own simple method. Result: a simpler method with less features, and thus: loss of reusable functionality.

How to solve this? Learn to detect when a method becomes a go-to place where to add functionality. An $options array attribute is good indicator to detect that. Try to refactor by moving the implementation to the child classes. Let them take care of their own use cases, like sorting. If sorting needs to be added in more than one repository, just duplicate some code for now. Don’t abstract right away. When you’re done duplicating code and everything works you can start adding specialized classes for sorting and using those to hold sorting logic. Remember : single responsibility? Ideally it should only take a few lines to inject sorting behavior into any repository.

Abstract class that defines dependencies for all of its children

The second example is a base implementation which defines dependencies for its subclasses, even though it doesn’t use these dependencies in the abstract class.

This is a base class that allows subclasses to dump data from a repository into a CSV file. This is reuseable, but what if you ever need to dump data that is not originating from a repository, but data from somewhere else, like a list of files in a directory? This base class knows about the CSV writer class, the repository and the transformer. Too much, right?

How to solve this? Traits to the rescue! What every CSV exporting class needs is access to the SplFileObject, and the Writer object (from the league/csv project). All else is an implementation detail. Thus we shall create a trait that holds both a $file and a $writer property.

Further more, initializing these properties is a burden, so the trait may provide a way to do so with a setup method.

In case the class that uses the trait doesn’t want to format the data for the CSV export itself, it may pass on a transformer which has some headers to insert on the first row of the file. Enter one last simple reusable method in the trait:

Now it’s easy for an exporter class to use the league/csv Writer class. It can do so by using the trait.

It’s as simple as that. Every class and trait is now reusable, testable and decoupled.

Note: these examples are boiled down from real world examples. They are working solutions for the problems they solve. Though, when requirements change, some of these solutions aren’t as easy to adapt as I would like. That’s why I’m merely trying to show how to write more decoupled code that is even more reusable and more flexible for the continually changing needs of projects.

When are abstract classes OK?

It is OK to create partial implementations for interfaces that have some method definitions that can have an implementation that is OK for 90+% of the interface implementations.

This is league/event‘s ListenerInterface. For almost every single Listener implementation out there, the isListener method should have the same implementation. Thus the league/event package comes with this AbstractListener class:

If, for some reason, you want to reuse an existing class (that already is part of its own inheritance tree and thus has its own parent class) to become a listener, and implement the ListenerInterface, you will need to implement the isListener method yourself. Alternatively you can create and reuse a simple trait like this:

Recap

  • A method in an abstract class that keeps growing with every additional use case is bad. It needs refactoring.
  • Abstract classes which have constructors are an antipattern.
  • If you are stuck with an existing inheritance tree: traits (for partial implementations) might be the answer to your problems.

Cheers!