This is a sequel to my earlier article "Using a decorator pattern to reduce inappropriate code complexity". You'd better go breeze through that before reading this one, otherwise this one won't make too much sense.
One of my colleagues read the above article and said "yeah good... but I wonder why one would do that over simple inheritance?" Good question. Especially as he is my boss, so all his questions are good (like I said: he also reads this blog ;-). My prepared answer was that the decorator pattern was an implementation representation of an interface, and accordingly it can kinda reflect multiple inheritance (albeit by composition) where an inheritance-based approach only allows a single inheritance chain: and that will get clumsy quickly. Note that I am not suggesting that the decorator pattern actually reflects a multiple inheritance pattern, I'm just using it as a comparative metaphor. I think it's reasonable.
In the previous article I showed how to simplify and focus the implementations of a UserRepository and a LoggedUserRepository (it writes to a log as well as making a given repo call), and a CachedUserRepository (it caches the repo call). And, indeed from there it was easy to decorate the UserRepository with both logging and caching, to effect either a LoggedCachedUserReppository, or a CachedLoggedUserRepository. If you see the subtle difference there: it's what order the ancillary operations take place in. This was in lieu of having all the caching and logging code baked into the UserRepository, which I think is less-than-ideal design, makes testing harder, and also presupposed an implemenation (which had already been demonstrated - in our case - to be a bad supposition, hence the origin of this investigation).
Code-wise, I had this:
class UserRepository implements RepositoryInterface {
public function getById($id) {
return (object) [
"id" => $id,
"firstName" => "Number $id",
"recordAccessed" => new \DateTime()
];
}
}
And the LoggedRepository used to decorate a UserRepository with logging might be like this:
class LoggedRepository implements RepositoryInterface {
private $repository;
private $loggerService;
public function __construct(RepositoryInterface $repository, LoggerServiceInterface $loggerService) {
$this->repository = $repository;
$this->loggerService = $loggerService;
}
public function getById($id) {
$this->loggerService->logText("$id requested");
$object = $this->repository->getById($id);
return $object;
}
}
The key points are that the UserRepository does all the data-getting, the LoggedRepository just decorates that with some logging. But each repo just plays to its strengths, and lets the other one get on with its own strength. That's the decorator pattern. The CachedRepository was analogous to the Logged~ one, just doing caching instead of logging. And to get both a logged & cached or a cached & logged UserRepository, it's just a matter of a further layer of decoration. Roughly like a matryoshka doll. Of sorts. Anyway: there's that whole other article about that.
So what about using inheritance instead? Let's swap out the repository type here to just a PersonRepository (the reason for this is solely cos all this code is in the same namespace, so I had already used the User~ metaphor). The PersonRespository is the same as the User one:
class PersonRepository implements RepositoryInterface {
public function getById($id) {
return (object) [
"id" => $id,
"firstName" => "Number $id",
"recordAccessed" => new \DateTime()
];
}
}
(I'm still just faking the actual data-fetch operation, as it's irrelevant here. But imagine
getById() gets a user via its ID from some data store).One could consider that a LoggedPersonRepository is a specialisation of UserRepository: it's for the same purpose, it just throws logging into the mix:
class LoggedPersonRepository extends PersonRepository {
protected $loggerService;
public function __construct(LoggerServiceInterface $loggerService) {
$this->loggerService = $loggerService;
}
public function getById($id) {
$this->loggerService->logText("$id requested");
$object = parent::getById($id);
return $object;
}
}
One can accurately say a LoggedPersonRepository IS A PersonRepository, so the inheritance seems sound, and if one thinks through scenarios, I think it passes a Liskov substitution principle test as well. So the design is - thusfar - sound. And looking at the code, this is actually more simple than the decorator pattern version because there's no need for this LoggedPersonRepository to take a PersonRepository argument, as it is a PersonRepository.
And it's samesame for a CachedPersonRepository, but I'll not bore you with the implementation detail. I'm guessing you can see what I mean.
But what about when we do add caching to our repositories? We'd need two more classes:
class CachedPersonRepository extends PersonRepository {
protected $cacheService;
public function __construct(CacheServiceInterface $cacheService) {
$this->cacheService = $cacheService;
}
public function getById($id) {
if ($this->cacheService->isCached($id)) {
return $this->cacheService->get($id);
}
$object = parent::getById($id);
$this->cacheService->put($id, $object);
return $object;
}
}
And:
class CachedLoggedPersonRepository extends LoggedPersonRepository {
protected $cacheService;
public function __construct(LoggerServiceInterface $loggingService, CacheServiceInterface $cacheService) {
parent::__construct($loggingService);
$this->cacheService = $cacheService;
}
public function getById($id) {
if ($this->cacheService->isCached($id)) {
return $this->cacheService->get($id);
}
$object = parent::getById($id);
$this->cacheService->put($id, $object);
return $object;
}
}
IE: CachedPersonRepository is an entirely different class from a CachedLoggedPersonRepository. This is still sound inheritance, and stands up to the LSP test too; so from that side of things, the design is still "valid". This doesn't make it optimal though. Also if we wanted to just have the caching, or the caching just higher in the hierarchy than the logging? Two more classes again.
And say you then want to add an encryption layer? That's potentially another five different class variations:
- EncrytedPersonRepository
- EncryptedLoggedPersonRepository
- EncryptedCachedPersonRepository
- EncryptedCachedLoggedPersonRepository
- EncryptedLoggedCachedPersonRepository
I think my example of a LoggedCachedEncryptedPersonRepository is slightly egregious, and I don't want to use an appeal to extremes to make my case here. However I think - all things being equal - using the decorator pattern instead of inheritance to solve this sort of thing is going to be a more robust way of preempting potential scaling requirements; whilst still being a simple and recognised solution; and an easy way to facilitate a possible future situation, whilst not in any way actually going down the rabbit-hole of actually writing any code for that potential future.
All the code and sanity checks of the behaviour can be found in my github account: decorator.local.
Righto.
--
Adam
