Via G. Falcone 5, Pollenza (MC), Italy
+39 0733 203595

How to decouple your components and improve testability and code reuse: a practical example

Today we are going to take into account a real example I was working on in the last days. It’s a quite simple case study that does not involve any knowledge about our software or domain so is perfect to show how, with a little bit of work, you can take strongly coupled classes and make them decoupled, reusable and more testable.

Scenario

Basically we wanted to create a class that takes an Entity representing a File (named FileEntity into following snippets) and apply a watermark on it through a service (named WatermarkGenerator). It’s not a pain-in-the-back-task, is it? Also is worth to note that in this first phase we want to handle only .pdf files.

To reach our goal, we create a WatermarkGenerator class that provides API to watermark files, PdfWatermarker – used into WatermarkGenerator – that applies watermark and PdfWatermarkText (not reported here) used to process watermark text.

It’s important to note that all namespaces, use directive and other parts of code not reported (including PHPDocs) are considered useless (and inopportune) for full comprehension. Also this process did not follow a TDD/BDD approach as this refactor involved a pre-existent code (that you will find below).

Existent code

EntityFileManager is just a service that helps us to abstract FileSystem (remote, local, etc) and provide facilities to access and write files. It’s not interesting for this article but we need to include it as takes part into refactor process.

As you can see here there are two huge design problems with WatermarkGenerator. As a matter of fact it is strictly coupled to FileEntity Рso, basically, you cannot reuse it with any other entity representing a file Рand it also uses EntityManager facilities; in particular this broke SOLID principle.
Also PdfWatermarker is strictly coupled to FileEntity

 

Start

Single responsibility principle … what?

To “master” this principle first question you have ALWAYS to ask yourself is

What this class is supposed to do?

If you can’t answer without using “and” word, well, your class is breaking SRP.

In this example, an answer to the question would be

This class is used for watermark a file and persist it to db

Refactor list

  • Decouple WatermarkGenerator and PdfWatermarker from FileEntity.
  • Follow SOLID principle and separate watermarking of FileEntity from its persist operation.

Step 1 – Decoupling

A possible solution for decoupling WatermarkGenerator and PdfWatermarker from FileEntity, is to create a generic WatermarkableEntity (that is pretty much a container for our objects) that implements an interface named WatermarkableElementInterface and pass it to WatermarkGenerator and PdfWatermarker.

WatermarkableEntity will contain FileEntity and provide a lot of abstraction on it (we will see how below). But, if is true that PdfWatermarker just needs to call getSignaturePosition() method regardless the class of file entity, that’s not fulfill WatermarkGenerator needs. In fact, based on file entity actual class (FileEntity in our example, but we will need to watermark also other entity types), we could need to perform tasks that are specific to it or – more common – we need a different way to invoke methods. For this reason we introduce a EntityFileWatermarkGenerator that extends WatermarkGenerator.

This is the code

As you can see a client (PdfWatermark) needs to know only what methods to call to meet its needs. No details about concrete classes.

Introducing FileEntityWatermarkGenerator.php

This works following this logic: all operations relative to FileEntity (pre and post watermark operations) are done right here but all “common” watermark operations are done through parent:: calls (WatermarkGenerator). You also might have noticed that type hinting in this scenario is not possibile; we will tackle it later.
Let’s see how WatermarkGenerator and PdfWatermarker are changed

Decouple

As you might have noticed above we can pass pretty much every type of entity file type to WatermarkGenerator as them will be enveloped into an external object implementing an interface. This is a huge advantage as we’re not declaring actual class of entity file anywhere in WatermarkGenerator.

Looking at these snippets of code we noticed that inheritance between FileEntityWatermarkGenerator and WatermarkGenerator could lead to inflexible scenarios (like every time you choose to use inheritance) and is also difficult to read what’s going on when FileEntityWatermarkGenerator‘s watermark() function is invoked. Moreover you can notice that watermark() can’t take advantage of type hint feature for argument passing due to inheritance (we can always slightly change names, it’s true, but this is just an example of kind of things you should keep an eye on with inheritance).

Composition over inheritance

We will not discuss here the benefits of composition over inheritance (you can query google and choose a “random” link; them are all just good) but we choose to follow the “rule” and implement a collaboration¬†between FileEntityWatermarkGenerator and WatermarkGenerator.

Code changes as follows

CompositionOverInheritance

Step 2 – Single Responsibility Principle (SRP)

Second phase expects to take out EntityManager and logic marked as *1 into code snippets, as those functionalities aren’t relevant to FileEntityWatermarkGenerator. This kind of operation is really easy to perform and understand.

Now this seems to fit well into SRP definition. Isn’t it?

Here will not be reported persistence code as it is straightforward as we invoke watermark() function and perform operations that were into saveWatermarkedFile() function elsewhere (typically into a service called *Manager where * stands for actual type of manager).

We are now very close to our goal but it is important to notice that WatermarkGenerator yet relies onto EntityFileManager. This should be a common class, but that way we are implicitly saying that every concrete entity file class should have its physical file onto file system. And what about, tomorrow, we decide to introduce a new WatermarkableEntity type that relies onto streams (or whatever?). Furthermore, why should WatermarkerGenerator be the one who keeps logic about file retrieve and write? That sounds also like a violation of SRP.

So … where to migrate this collaborator and the logic about file system?
Our answer is “into FileEntityWatermarkableElement“, because we are sure that this element will always relies onto filesystem and because that way none of the clients who use it need to know anything about implementation details. Sounds like a very good solution.

 

It’s important to notice that getInputPath() and getOutputPath() should be included into WatermarkableElementInterface contract as are invoked by class that relies on WatermarkableElementInterface (not reported here for shortness; e.g.: WatermarkGenerator that now has not EntityFileManager injected anymore)

Final

Conclusions

Classes are not strictly coupled anymore. They can be combined to obtain different ways to retrieve a file, place a watermark on it and write back content onto any destination (file system, stream, and so on) just by creating a new version of <ConcreteEntityFile>WatermarkableElement and, if “domain logic” is present, a new version of <ConcreteEntityFile>WatermarkerGenerator that should be used with common WatermarkGenerator.

If you did not follow a pure BDD/TDD approach (as I did, because I found already written code to modify) and tried to write tests onto first version of code, you would probably noticed that a lot of mocks and stubs were involved into this process, making test classes very messy! If you take a look to post-refactor code you probably understand at a first and shallow glance that code is now easier to test.

Finally, one possibile upgrade to this code could be a factory introduction to obtain a WatermarkerGenerator able to watermark not only .pdf but a lot of different types of file (so, remove dependency between WatermarkerGenerator and PdfWatermarker).

Leave a reply