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.
abstract class AbstractRepository
{
public function __construct(Model $models)
{
$this->models = $models;
}
public function get(array $options = [])
{
if (isset($options['sort'])) {
//...
}
if (isset($options['filters'])) {
//...
}
return $this->models->get();
}
}
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.
abstract class CsvExporter
{
protected $repository;
protected $file;
protected $writer;
protected $parameters;
public function __construct(RepositoryInterface $repository, TransformerInterface $transformer, array $parameters)
{
$this->repository = $repository;
$this->parameters = $parameters;
$this->file = new SplTempFileObject();
$this->writer = Writer::createFromFileObject($this->file);
$this->writer->insertOne($transformer->getHeaders());
$this->writer->addFormatter([$transformer, 'transform']);
}
}
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.
trait CsvWriter
{
private $file;
private $writer;
}
Further more, initializing these properties is a burden, so the trait may provide a way to do so with a setup
method.
trait CsvWriter
{
private $file;
private $writer;
private function setup()
{
$this->file = new SplTempFileObject();
$this->writer = Writer::createFromFileObject($this->file);
}
}
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:
private function setTransformer(TransformerInterface $transformer)
{
$this->writer->insertOne($transformer->getHeaders());
$this->writer->addFormatter([$transformer, 'transform']);
}
Now it’s easy for an exporter class to use the league/csv
Writer class. It can do so by using the trait.
class UserExporter
{
use CsvWriter;
public function __construct(User $users, UserTransformer $transformer)
{
$this->setup();
$this->setTransformer($transformer);
$this->users = $users;
}
public function getFile()
{
$this->users->chunk(100, function ($users) {
$this->writer->insertAll($users);
});
return $this->file;
}
}
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.
namespace League\Event;
interface ListenerInterface
{
/**
* Handle an event.
*
* @param EventInterface $event
*
* @return void
*/
public function handle(EventInterface $event);
/**
* Check whether the listener is the given parameter.
*
* @param mixed $listener
*
* @return bool
*/
public function isListener($listener);
}
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:
namespace League\Event;
abstract class AbstractListener implements ListenerInterface
{
/**
* {@inheritdoc}
*/
public function isListener($listener)
{
return $this === $listener;
}
}
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:
trait PartialListener
{
/**
* {@inheritdoc}
*/
public function isListener($listener)
{
return $this === $listener;
}
}
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!
Member discussion