DEV Community: Samuele Lilli

Composition over inheritance: tackle the "Template Method"

Samuele Lilli — Wed, 24 Nov 2021 16:33:01 +0000

In this article we’re gonna talk about composition over inheritance but not approaching this topic from a theoretical point of view (you can find dozens of good articles online by googling the topic), but instead by providing an example of something that in literature is a well-know pattern: Template Method.

Template Method

Template method is a behavioral design pattern conceived to encapsulate an algorithm skeleton where some methods (at least one) are declared abstract in order to let concrete classes extend the "template" and provide a concrete implementation. This pattern is an example of Inversion Of Control, which I’ve already written about. Based on its nature, Template Method strongly relies on the inheritance principle, but despite this, we can reach pretty much the same results by using composition.

Disclaimer

With this article I’m not advocating that one approach is better than the other; as a matter of fact I’ll try to highlight pros and cons of both. This article was written because I was searching for alternative "compositive" implementations of "canonical" design patterns and I didn’t found a single one for Template Method.

Example

Based on the example linked above, here’s a slightly-modified code:

abstract class UserRegister 
{
   private $em;

   public function __construct(EntityManagerInterface $em) 
   {
       $this->em = $em;
   }

   final public function register(User $user): void 
   {
       $user->enable();
       if (!$this->sendVerification($user)) {
         return;
       }

       $this->em->persist($user);
       $this->em->flush();
   }

   protected abstract function sendVerification(User $user): bool;
}

class EmailVerificationUserRegister extends UserRegister 
{
   protected function sendVerification(User $user): bool
   {
       $email = $user->getEmail();

       return $this->sendEmail($email);
   }

   private function sendEmail(string $email): bool
   {
      // omitted: sending email code
   }
}

class PhoneVerificationUserRegister extends UserRegister 
{
   protected function sendVerification(User $user): bool
   {
       $phone = $user->getPhoneNr();

       return $this->sendSms($phone);
   }

   private function sendSms(string $phone): bool
   {
     // omitted: sending sms code
   }
}

class NoVerificationUserRegister extends UserRegister 
{
   protected function sendVerification(User $user): bool
   {
       $user->verified();

       return true;
   }
}

Above is shown the template method code. Below, the "client" one

class Client
{
  private $register;

  public function __construct(UserRegister $register)
  {
    $this->register = $register;
  }

  public function register(User $user)
  {
    // do something before registration
    $this->register->register($user);
    // do something after registration
  }
}

$client = new Client(new EmailVerificationUserRegister());
$client->register($new User());

EmailVerificationUserRegister was randomly choosen and you can both choose another concrete implementation, or configure it with dependency injection mechanism (that’s not reported here in order to keep the complexity as low as possible).

Composition

We have two alternatives (perhaps there could be more but those are what I found): strategy pattern and closure as callback. Starting from the latter - which is the worst of two but I still wanted share - I ended up with the following code:

class UserRegister 
{
   private $em;

   public function __construct(EntityManagerInterface $em) 
   {
       $this->em = $em;
   }

   final public function register(User $user, $sendVerification): void 
   {
       $user->enable();
       if (!is_callable($sendVerification) {
         return;
       }

       $verificationHasBeenSent = call_user_func_array($sendVerification, [$user]);
       if (is_bool($verificationHasBeenSent) && $verificationHasBeenSent) {
         return;
       }

       $this->em->persist($user);
       $this->em->flush();
   }
}

Talking of closure as callback, a closure is an anonymous function (also called lambda) introduced back in PHP 5.3 and basically is a snippet of code (a function, indeed) that you don’t need to declare but just use "inline" and inherits the scope of where it is defined (unless explicitly binded to other context). In order to obtain the same result as the inheritance example, we should modify one of the concrete implementations as follows

interface UserRegisterInterface
{
  public function register(User $user): void;
}

class EmailVerificationUserRegister implements UserRegisterInterface
{
   private $register;

   public function __construct(UserRegister $register)
   {
     $this->register = $register;
   }

   public function register(User $user)
   {
     $this->register->register($user, [$this, 'sendVerification']);
   }

   private function sendVerification(User $user): bool
   {
       $email = $user->getEmail();

       return $this->sendEmail($email);
   }

   private function sendEmail(string $email): bool
   {
      // omitted: sending email code
   }
}

Pay attention to call_user_func_array and to the fact that UserRegister::register has no typehint on the callback. I could have done something like

class UserRegister 
{
   [...]
   final public function register(User $user, \Closure $sendVerification): void 
   {
       $user->enable();
       $verificationHasBeenSent = $sendVerification($user);
       if (is_bool($verificationHasBeenSent) && $verificationHasBeenSent) {
         return;
       }
       [...]
   }
}

class EmailVerificationUserRegister implements UserRegisterInterface 
{ 
  [...]
  public function register(User $user) 
  { 
    $this->register->register($user, function (User $user) { return $this->sendVerification($user); });
  } 
  [...]
}

but call_user_func_array is much safer. Moreover I needed to introduce UserRegisterInterface in order to use it in Client class (that I won’t show again because the only thing that changes is the typehint in its constructor).

The second way is to use something similar to a strategy pattern as follows:

interface VerificationStrategy
{
  public function sendVerification(User $user): bool;
}

class UserRegister 
{
   private $em;

   public function __construct(EntityManagerInterface $em) 
   {
       $this->em = $em;
   }

   final public function register(User $user, VerificationStrategy $strategy): void 
   {
       $user->enable();
       if (!$strategy->sendVerification($user)) {
         return;
       }

       $this->em->persist($user);
       $this->em->flush();
   }
}

class EmailVerificationUserRegister implements VerificationStrategy 
{ 
  public function sendVerification(User $user): bool 
  { 
    $email = $user->getEmail(); 

    return $this->sendEmail($email); 
  } 

  private function sendEmail(string $email): bool 
  { 
    // omitted: sending email code 
  } 
}

class Client 
{
  private $register; 

  public function __construct(UserRegister $register) 
  { 
    $this->register = $register; 
  } 

  public function register(User $user, VerificationStrategy $strategy) 
  { 
    // do something before registration 
    $this->register->register($user, $strategy); 
    // do something after registration 
  } 
} 

$client = new Client(); 
$client->register(new User(), new EmailVerificationUserRegister());

Conclusions

To summarize, beside the advantage of composition over inheritance that you can find pretty much everywhere on the web, I can take to your attention what follows:

With inheritance, you can inherit just from one class (at least in PHP). The example above is fine but can be hard to change if something changes in the behavior.
With inheritance, you need to test abstract class and all concrete classes as well: from my point of view, skipping concrete classes testing is just wrong as the extension is just an implementation detail. No tests on these classes means no tests for them at all.
With closures, you can’t ensure that parameters passed from the caller are consistent with function definition: imagine something changes in the way the register class invokes the callback or manages its results: you won’t notice as long as you don’t have functional/integration/e2e tests (that seems necessary if using closures). The return type could also be an issue; in fact I needed some extra control code in order to check the consistency.
With closures, in the first example, it’s also troublesome to refactor the method name as it is a string (even if modern IDEs could help you a lot in this, is just not fine).
With strategy you can easily adapt the code if the behavior changes. Moreoever you can have unit test that aren’t just a copy/paste of abstract class tests (in case of inheritance). Of course you need to a bit more "context switch" to understand how the code works but from my point of view, it’s a good tradeoff.

That’s all.

Happy coding! 🙂

Composer conflict, how can we use it?

Samuele Lilli — Fri, 19 Nov 2021 09:59:01 +0000

Today I found a very useful Composer schema package link option named conflict, which is useful when third-party code you rely on is too permissive for your needs.

At the time of writing, the conflict man page states

Lists packages that conflict with this version of this package. They will not be allowed to be installed together with your package.

Note that when specifying ranges like <1.0 >=1.1 in a conflict link, this will state a conflict with all versions that are less than 1.0 and equal or newer than 1.1 at the same time, which is probably not what you want. You probably want to go for <1.0 || >=1.1 in this case.

In order to explain how we can take advantage of this, we’ll use a concrete example.

The problem with a Guzzle dependency

At Madisoft we love open source and believe in its power, not only because we have third-party code on which our application is based upon (BTW, thanks to everyone who makes our daily job easier!), but also because we like to contribute and have a commitment to OSS.

One of the packages we use and help improve is BehatPageObjectExtension, an extension for Behat that incapsulates the PageObject pattern (we have written a blog post about PageObjectExtension, in Italian, here).

When looking at this pull request I’ve opened to add PHP 7.4 support to the library, you may notice a previous Travis CI build with failing test.

The reasons behind the issue are:

Dropping support for PHP older versions (check the PHP supported versions here)
Reflecting this change on .travis.yaml (a file that tells the CI platform how to create the environment to run your tests on)
Shifting the deps=low flag to the lowest supported PHP version in order to tell Composer to use the --prefer-lowest flag when installing the dependencies.

Pay attention to last point on the list: when working on OSS, it’s important to try to keep everyone’s code - that depends on your OSS - working (at least until a bump to a major version) so, in sight of this, it’s common to check if the lowest possible dependencies your code accepts will still make the tests pass.

Once we shifted the deps=low flag to PHP 7.2, we found the above issue. It was already there, but we hadn’t noticed as the flag was only on PHP 7.1, meaning that tests never ran against PHP 7.2 with lowest dependencies. As PHP 7.2 introduced a warning on certain count() usages, our code didn’t manage to pass the tests.

But if you look closely, you’ll probably notice that the warning was not raised by our code directly, but by third-party code we’re requiring with Composer:

Warning: count(): Parameter must be an array or an object that implements Countable in /home/travis/build/sensiolabs/BehatPageObjectExtension/vendor/guzzlehttp/guzzle/src/Handler/CurlFactory.php line 66

We found the related issue on the Guzzle GitHub repository and we noticed a fix had been released with version 6.3 of this library. Hurray, we can bump Guzzle version to 6.3 and overcome the problem! But looking at BehatPageObjectExtension’s composer.json we’ve soon realized that Guzzle isn’t a direct dependency managed by us. “It’s not a big deal” I thought as Guzzle was required by Goutte and we require Goutte through its Mink driver,so I expected to find some tagged version of Goutte and the Mink Driver with this requirements bump. Sadly I found it wasn’t the case as Guzzle is required in Goutte with ^6.0 (so, basically every version between 6 and 7) and this includes the versions with the warning described above. Therefore in our case the warning was displayed as a “side effect” of deps=low that requires indirectly Guzzle 6.0 as is the lowest dependency accepted.

Without discussing if it’s right or wrong to keep a version of third party code that could break everthing (take a look at this PR), we were suddenly at a crossroads: drop the deps=low and give up on this kind of tests or dig in the Composer manual and look for something that could possibly help us: you guessed right, conflict is what we needed (thanks to jakzal!).

Here are the details of the commit with conflict.

Conclusions

With conflict you can force the minimum version of a dependency you cannot control directly, avoiding headaches or third party code forks.

Hope this little story helped you to learn something new, personally I did!

Happy coding!

Bonus

In order to avoid accepting third-party code with well-known security issues you can take advantage of SecurityAdvisories by Roave, a library which uses conflict as shown in this article to block unsafe packages. Give it a look!

Avoid (status) flag

Samuele Lilli — Wed, 17 Nov 2021 14:08:03 +0000

Hi folks, DonCallisto here.

In this article we'll see why I personally don't like status flags and how to handle them in a more elegant way. We'll find out code written without flags which is semantically better and convey business rules way better. Curious? Keep reading!

Before start, I would like to be sure you understand what status flag are. Since I'm not aware of a literature definition for the concept, I come up with one (if someone finds it, please leave a comment and I'll be happy to edit with an accurate quote). When I talk about status flag I mean a class member which decree the current status of an object (class instance). For example think of a kanban board with several columns like "confirmed", "assigned", "in progress", "done" and "released". A task inside the kanban, in a given moment, can be pinned in one and only one of these columns. It follows that a task has a status, alternatively confirmed, assigned (to someone) and so on. That's what I mean when I use "status flag" locution. You can also think them in terms of enums. Having said that, I would not include binary flags in this definition as only two possible values can be assigned, like true or false, on or off, open or closed, and so on and so forth; further on should be clear why.

Going straight to the point, consider a piece of software responsible for blog's articles management. An article has a status depending on the point in time you look at it. In this example possible status are: draft, ready for review, reviewed and published. There's also a business rule we need to take into account and states that a status transition can happen only in a finite-state machine like fashion. An article is created in a draft status, then it can be moved to ready for a review, then when reviewed it acquire a reviewed status and only when reviewed it can be published. So we got a pretty straightforward state machine, where transitions happens in a one way direction from a state to the other as depicted below

To ease the understanding of the concept, Article was modeled as a simplified version of a real world scenario so we can focus on what matters the most leaving out the irrelevant details. For example I decided to avoid transitions like the one back from "ready for review" to "draft", I also kept out some status like "need edit" when a review is done but not accepted and - you'll see in no time - articles doesn't have a title or other attributes one expects to find in a real world scenario. These are only some of the simplification you can find reading on this post.

So far so good, context is set and we're now ready to go on. As Linus Torvalds said once... "Talk is cheap, show me the code!"

Consider the following PHP 8.1 classes (feel free to check out code directly in GitHub; please notice the link point to a specific commit and not to the whole repo, further on, when needed, I'll link other commits)

<?php

declare(strict_types=1);

namespace Domain\Article;

enum Status
{
    case DRAFT;
    case REVIEW;
    case REVIEWED;
    case PUBLISHED;

    public function isDraft(): bool
    {
        return $this == self::DRAFT;
    }

    public function isReview(): bool
    {
        return $this == self::REVIEW;
    }

    public function isReviewed(): bool
    {
        return $this == self::REVIEWED;
    }

    public function isPublished(): bool
    {
        return $this == self::PUBLISHED;
    }
}

<?php

declare(strict_types=1);

namespace Domain\Article;

use Domain\Article\Exception\ArticleException;
use Domain\User\User;

class Article
{
    private Status $status;

    private ?User $reviewer;

    private ?\DateTimeImmutable $publicationDate;

    public function __construct(private User $author, private string $content)
    {
        $this->status = Status::DRAFT;
        $this->reviewer = null;
        $this->publicationDate = null;
    }

    public function isInDraft(): bool
    {
        return $this->status->isDraft();
    }

    /**
     * @throws ArticleException
     */
    public function readyForReview(): void
    {
        if ($this->isReadyForReview()) {
            return;
        }

        if (!$this->isInDraft()) {
            throw ArticleException::cantBeReadyForReview();
        }

        $this->status = Status::REVIEW;
    }

    public function isReadyForReview(): bool
    {
        return $this->status->isReview();
    }

    /**
     * @throws ArticleException
     */
    public function reviewed(User $reviewer): void
    {
        if ($this->isReviewed()) {
            return;
        }

        if (!$this->isReadyForReview()) {
            throw ArticleException::cantBeReviewed();
        }

        if ($reviewer === $this->author) {
            throw ArticleException::reviewedByAuthor();
        }

        $this->status = Status::REVIEWED;
        $this->reviewer = $reviewer;
    }

    public function isReviewed(): bool
    {
        return $this->status->isReviewed();
    }

    /**
     * @throws ArticleException
     */
    public function publish(\DateTimeInterface $publicationDate): void
    {
        if ($this->isPublished()) {
            return;
        }

        if (!$this->isReviewed()) {
            throw ArticleException::cantBePublished();
        }

        $this->status = Status::PUBLISHED;
        $this->publicationDate = \DateTimeImmutable::createFromInterface($publicationDate);
    }

    public function isPublished(): bool
    {
        return $this->status->isPublished();
    }

    public function getAuthor(): User
    {
        return $this->author;
    }

    public function getContent(): string
    {
        return $this->content;
    }

    /**
     * @param string $newContent
     *
     * @throws ArticleException
     */
    public function changeContentTo(string $newContent): void
    {
        if (!$this->isInDraft()) {
            throw ArticleException::contentCantBeChanged();
        }

        $this->content = $newContent;
    }

    // We can also throw an exception...
    public function getReviewer(): ?User
    {
        return $this->reviewer;
    }

    // We can also throw an exception...
    public function getPublicationDate(): ?\DateTimeImmutable
    {
        return $this->publicationDate;
    }
}\

User and ArticleException were left out; you can find whole code on GitHub repo linked above.

Status is an enum, a new PHP feature you can take advantage of from PHP 8.1. Status is our status flag, the one you've read during introduction. Looking at Article, it's easy to find how I used Status and I'm pretty sure you, in your developer career, did wrote code in this fashion. No shame in that. Probably you're wondering what's wrong with Article class as it seems to model every business rule described before. The point is I'm using a single class to represent and model four different status, so four different behaviors, all in one class. You may argue that there's only one domain concept - other than Status that you can see pretty much as a value object as it's immutable and its identity is verified not by its id but by its values - and it is Article, but from my point of view, as long as Article should behave differently depending on its status, it should be modeled as different objects. To support latter statement, consider what follows.

publicationDate attribute must be nullable as it has meaning only for a certain Status. This is a smell for little cohesion. Moreover as publicationDate is nullable, getPublicationDate must be able to return null or, alternatively, must throw an exception if publicationDate is null. Either chances are bad as the first forces the client to explicitly check for null values, whereas the second forces the client to catch possible exceptions and can be fuzzy if seen from outside (why on the world publicationDate retrieval can lead to an exception?). In addition, and as a side effect, publicationDate concept has no meaning in certain situation. To prove that, go and ask your domain experts something like "what's the publication date of a draft/ready for review/reviewed article?". No surprise if they'll be puzzled. The same goes for reviewer.
There's a lot handling for public API. For instance, look at readyForReview public method (the same goes for all transitional method). Internally we are forced to put an extra effort to convey and guarantee a domain rule for Status changes.
Public API is not telling anything about the business. Leaving out is-ser methods in Article and looking at the class from the outside through its API we have readyForReview, reviewed, publish, changeContentTo, getReviewer and getPublicationDate. For a client it might seem possible to change Article status to any of the possible status regardless of its current one. Anybody which knows well domain rules is forced to look into Article code to find what can go possibly wrong. Even if domain invariants are respected, here we're hiding them and communicating something wrong about Article nature. Talking about getReviewer and getPublicationDate, I've already pointed out what's the issue.
Think of a statistics service (a class) designed to provide daily, weekly, monthly and yearly unique readers for a (published) article. Can you imagine its unique public method? It would be something like public function getReaders(Article $article): int. Again looking at this method some questions could be raised; one above all is what to return when an article is not published. Should we return 0 that's equivocal with the concept of "no readers", or should we return null, meaning statistics cannot be retrieved? Should we raise an exception if article is not published? From the point of view of a newcomer, whatever path you take, is it clear the concept behind the return value? Is it clear only Status::PUBLISHED articles will be manageables?
Think about entity retrieval. Let's pretend we wish to retrieve only Status::DRAFT articles. You need to add an extra WHERE condition when querying. Yes of course it's not dramatic from performance perspective, but this is true as long as you store few records. Reading this article you may find that your queries could have nearly 60% of overhead (when millions records stored).
Think about a new attribute which has meaning only for some or just one status but not for the whole object (like publicationDate). Whoever modify Article must put attention when introducing this kind of change as a forgetfulness could lead to inconsistent API and data (see it like adding a method or an attribute to a class where it does not belongs. Yes, it won't happen when dealing with unrelated classes but could happen in this specific case).

These are some points against status flags. Maybe there could be more, or maybe you won't consider any of these problematic. As usual your mileage may vary.

An alternative

Taking up one of previous statements

"The point is I'm using a single class to represent and model four different status, so four different behaviors, all in one class."

Let's see how code can be changed to express better domain concepts (GitHub code here)

<?php

declare(strict_types=1);

namespace Domain\Article;

use Domain\User\User;

interface ArticleInterface
{
    public function getContent(): string;

    public function getAuthor(): User;
}

<?php

declare(strict_types=1);

namespace Domain\Article;

use Domain\User\User;

class DraftArticle implements ArticleInterface
{
    public function __construct(private User $author, private string $content)
    {

    }

    public function getAuthor(): User
    {
        return $this->author;
    }

    public function getContent(): string
    {
        return $this->content;
    }

    public function changeContentTo(string $newContent): void
    {
        $this->content = $newContent;
    }

    public function readyForReview(): ReadyForReviewArticle
    {
        return new ReadyForReviewArticle($this);
    }
}

<?php

declare(strict_types=1);

namespace Domain\Article;

use Domain\User\User;

class ReadyForReviewArticle implements ArticleInterface
{
    private User $author;

    private string $content;

    public function __construct(DraftArticle $draft)
    {
        $this->author = $draft->getAuthor();
        $this->content = $draft->getContent();
    }

    public function getAuthor(): User
    {
        return $this->author;
    }

    public function getContent(): string
    {
        return $this->content;
    }

    public function reviewed(User $reviewer): ReviewedArticle
    {
        return new ReviewedArticle($this, $reviewer);
    }
}

<?php

declare(strict_types=1);

namespace Domain\Article;

use Domain\User\User;

class ReviewedArticle implements ArticleInterface
{
    private User $author;

    private string $content;

    public function __construct(ReadyForReviewArticle $readyForReview, private User $reviewer)
    {
        $this->author = $readyForReview->getAuthor();
        $this->content = $readyForReview->getContent();
    }

    public function getAuthor(): User
    {
        return $this->author;
    }

    public function getReviewer(): User
    {
        return $this->reviewer;
    }

    public function getContent(): string
    {
        return $this->content;
    }

    public function publish(\DateTimeInterface $publicationDate): PublishedArticle
    {
        return new PublishedArticle($this, $publicationDate);
    }
}

<?php

declare(strict_types=1);

namespace Domain\Article;

use Domain\User\User;

class PublishedArticle implements ArticleInterface
{
    private User $author;

    private string $content;

    private \DateTimeImmutable $publicationDate;

    public function __construct(ReviewedArticle $reviewed, \DateTimeInterface $publicationDate)
    {
        $this->author = $reviewed->getAuthor();
        $this->content = $reviewed->getContent();
        $this->publicationDate = \DateTimeImmutable::createFromInterface($publicationDate);
    }

    public function getAuthor(): User
    {
        return $this->author;
    }

    public function getContent(): string
    {
        return $this->content;
    }

    public function getPublicationDate(): \DateTimeImmutable
    {
        return $this->publicationDate;
    }
}

Previous code was split into four classes and an interface. The interface is helpful for circumstances when an article should be handled regardless its Status. Moreover we got rid of Status object and exception object as Status is incapsulated directly into object types. New classes has narrower public APIs and domain concepts are better defined and communicated to the outside. New developers can easily understand what domain rules are when talking about transitions, no exceptions or nullable values could be raised or returned, narrow services API (you can choose to accept only an article in a specific status, if needed), and so on and so forth. Finally, if a new attribute and/or API method should be added it's easier not to mess up other classes with senseless concepts.

Of the critical points showed above, only questionable one is the retrieval. If it is true that a WHERE condition could lead to worse performance, it's also true that retrieving all articles (in any state) could require up to four queries (unless you use some kind of technique like single table inheritance or similar which can bring out some other kind of issues).

A practical example

As I didn't want to left the reader with only a "conceptual" solution where these objects weren't shown "in action", I've decided to develop a nearly real world scenario. Let's say we would like to export the whole list of articles in different formats. When using a single object it is child's play as all methods reside in the same class and only one class is involved, but how we can do this when working with four different objects, considering that every class has its own methods and type?

See the code

An interface is defined for articles (not entities one, but a sort of DTO as I would not wanted to bloat domain class with application concepts) to accept a Visitor, following the Visitor Pattern

<?php

namespace Application\Exporter\Article\Visitor;

use Application\Exporter\Model\Article\ExportableDraftArticle;
use Application\Exporter\Model\Article\ExportablePublishedArticle;
use Application\Exporter\Model\Article\ExportableReadyForReviewArticle;
use Application\Exporter\Model\Article\ExportableReviewedArticle;

interface ArticleVisitor
{
    public function visitDraft(ExportableDraftArticle $draftArticle): void;

    public function visitReadyForReview(ExportableReadyForReviewArticle $readyForReviewArticle): void;

    public function visitReviewed(ExportableReviewedArticle $reviewedArticle): void;

    public function visitPublished(ExportablePublishedArticle $publishedArticle): void;

    public function visitResult(): mixed;
}

<?php

declare(strict_types=1);

namespace Application\Exporter\Article;

use Application\Exporter\Article\Visitor\ArticleVisitor;

interface ExportableArticle
{
    public function accept(ArticleVisitor $visitor): void;
}

Then two types of visitors are defined: one for array format and the other for json format, both under ArticleVisitor interface.

<?php

declare(strict_types=1);

namespace Application\Exporter\Article\Visitor;

use Application\Exporter\Model\Article\ExportableDraftArticle;
use Application\Exporter\Model\Article\ExportablePublishedArticle;
use Application\Exporter\Model\Article\ExportableReadyForReviewArticle;
use Application\Exporter\Model\Article\ExportableReviewedArticle;
use JetBrains\PhpStorm\ArrayShape;

class ArticleArrayVisitor implements ArticleVisitor
{
    #[ArrayShape([
        'author' => [
            'username' => 'string',
            'email' => 'string',
        ],
        '?reviewer' => [
            'username' => 'string',
            'email' => 'string',
        ],
        'content' => 'string',
        '?publicationDate' => \DateTimeImmutable::class,
        'status' => 'string'
    ])]
    private array $visitResult = [];

    public function visitDraft(ExportableDraftArticle $draftArticle): void
    {
        $this->visitResult = [
            'author' => [
                'username' => $draftArticle->getAuthor()->getUsername(),
                'email' => $draftArticle->getAuthor()->getEmail(),
            ],
            'content' => $draftArticle->getContent(),
            'status' => $draftArticle->getStatus()->value,
        ];
    }

    public function visitReadyForReview(ExportableReadyForReviewArticle $readyForReviewArticle): void
    {
        $this->visitResult = [
            'author' => [
                'username' => $readyForReviewArticle->getAuthor()->getUsername(),
                'email' => $readyForReviewArticle->getAuthor()->getEmail(),
            ],
            'content' => $readyForReviewArticle->getContent(),
            'status' => $readyForReviewArticle->getStatus()->value,
        ];
    }

    public function visitReviewed(ExportableReviewedArticle $reviewedArticle): void
    {
        $this->visitResult = [
            'author' => [
                'username' => $reviewedArticle->getAuthor()->getUsername(),
                'email' => $reviewedArticle->getAuthor()->getEmail(),
            ],
            'reviewer' => [
                'username' => $reviewedArticle->getReviewer()->getUsername(),
                'email' => $reviewedArticle->getReviewer()->getEmail(),
            ],
            'content' => $reviewedArticle->getContent(),
            'status' => $reviewedArticle->getStatus()->value,
        ];
    }

    public function visitPublished(ExportablePublishedArticle $publishedArticle): void
    {
        $this->visitResult = [
            'author' => [
                'username' => $publishedArticle->getAuthor()->getUsername(),
                'email' => $publishedArticle->getAuthor()->getEmail(),
            ],
            'content' => $publishedArticle->getContent(),
            'publicationDate' => $publishedArticle->getPublicationDate(),
            'status' => $publishedArticle->getStatus()->value,
        ];
    }

    public function visitResult(): array
    {
        return $this->visitResult;
    }
}

<?php

declare(strict_types=1);

namespace Application\Exporter\Article\Visitor;

use Application\Exporter\Model\Article\ExportableDraftArticle;
use Application\Exporter\Model\Article\ExportablePublishedArticle;
use Application\Exporter\Model\Article\ExportableReadyForReviewArticle;
use Application\Exporter\Model\Article\ExportableReviewedArticle;

class ArticleJsonVisitor implements ArticleVisitor
{
    private string $visitResult;

    public function __construct(private ArticleArrayVisitor $visitor)
    {

    }

    public function visitDraft(ExportableDraftArticle $draftArticle): void
    {
        $this->visitor->visitDraft($draftArticle);

        $this->visitResult = json_encode($this->visitor->visitResult(), JSON_THROW_ON_ERROR);
    }

    public function visitReadyForReview(ExportableReadyForReviewArticle $readyForReviewArticle): void
    {
        $this->visitor->visitReadyForReview($readyForReviewArticle);

        $this->visitResult = json_encode($this->visitor->visitResult(), JSON_THROW_ON_ERROR);
    }

    public function visitReviewed(ExportableReviewedArticle $reviewedArticle): void
    {
        $this->visitor->visitReviewed($reviewedArticle);

        $this->visitResult = json_encode($this->visitor->visitResult(), JSON_THROW_ON_ERROR);
    }

    public function visitPublished(ExportablePublishedArticle $publishedArticle): void
    {
        $this->visitor->visitPublished($publishedArticle);

        $this->visitResult = json_encode($this->visitor->visitResult(), JSON_THROW_ON_ERROR);
    }

    public function visitResult(): string
    {
        return $this->visitResult;
    }
}

ArticleJsonVisitor is defined as a decorator for ArticleArrayVisitor.

I would not report ExportableArticle implementations as you can find whole code in GitHub repo.

Lastly here's the exporter

<?php

declare(strict_types=1);

namespace Application\Exporter;

use Application\Exporter\Article\ExportableArticle;
use Application\Exporter\Article\Visitor\Factory\VisitorAbstractFactory;

class Exporter
{
    public function export(VisitorAbstractFactory $visitorAbstractFactory, ExportableArticle ...$exportableArticles)
    {
        $visitor = $visitorAbstractFactory->createVisitor();

        $results = [];
        foreach ($exportableArticles as $exportableArticle) {
            $exportableArticle->accept($visitor);
            $results[] = $visitor->visitResult();
        }

        $aggregator = $visitorAbstractFactory->createAggregator();

        return $aggregator->aggregate($results);
    }
}

where VisitorAbstractFactory is a factory used to get both visitor and aggregator (find it, again, on GitHub repo) in an Abstract Factory pattern fashion. As you can see we're using ExportableArticle that's the corresponding (more or less) for domain's ArticleInterface; just to show how easy is to export all kind of objects as it would have been with a single Article class.

Lastly, a working example

<?php

declare(strict_types=1);

include('vendor/autoload.php');

use Application\Exporter\Article\Visitor\Factory\ArrayAuthorAggregatedVisitorFactory;
use Application\Exporter\Article\Visitor\Factory\ArrayVisitorFactory;
use Application\Exporter\Article\Visitor\Factory\JsonAuthorAggregatedVisitorFactory;
use Application\Exporter\Article\Visitor\Factory\JsonVisitorFactory;
use Application\Exporter\Exporter;
use Application\Exporter\Model\Article\ExportableDraftArticle;
use Application\Exporter\Model\Article\ExportablePublishedArticle;
use Application\Exporter\Model\Article\ExportableReadyForReviewArticle;
use Application\Exporter\Model\Article\ExportableReviewedArticle;
use Domain\Article\DraftArticle;
use Domain\Article\PublishedArticle;
use Domain\Article\ReadyForReviewArticle;
use Domain\Article\ReviewedArticle;
use Domain\User\User;

$donCallisto = new User('DonCallisto', 'samuele.lilli@gmail.com');
$johnDoe = new User('JohnDoe', 'john.doe@gmail.com');
$otsillacNod = new User('OtsillacNod', 'samuele.lilli@madisoft.it');

$a1 = new ExportableDraftArticle(
    new DraftArticle($donCallisto, 'foo')
);
$a2 = new ExportableReadyForReviewArticle(
    new ReadyForReviewArticle(
        new DraftArticle($donCallisto, 'foo')
    )
);
$a3 = new ExportableReviewedArticle(
    new ReviewedArticle(
        new ReadyForReviewArticle(
            new DraftArticle($donCallisto, 'foo'),
        ),
        $otsillacNod
    )
);

$a4 = new ExportablePublishedArticle(
    new PublishedArticle(
        new ReviewedArticle(
            new ReadyForReviewArticle(
                new DraftArticle($donCallisto, 'foo'),
            ),
            $otsillacNod
        ),
        new \DateTime()
    )
);

$a5 = new ExportableDraftArticle(
    new DraftArticle($johnDoe, 'foo')
);
$a6 = new ExportableReadyForReviewArticle(
    new ReadyForReviewArticle(
        new DraftArticle($johnDoe, 'foo')
    )
);

echo 'Start array exporting...' . PHP_EOL;
$exporter = new Exporter();
$result = $exporter->export(new ArrayVisitorFactory(), $a1, $a2, $a3, $a4, $a5, $a6);
echo 'Results: ' . PHP_EOL;
echo print_r($result, true) . PHP_EOL;
echo 'Finish array exporting' . PHP_EOL;

echo 'Start json exporting...' . PHP_EOL;
$exporter = new Exporter();
$result = $exporter->export(new JsonVisitorFactory(), $a1, $a2, $a3, $a5, $a6);
echo 'Results: ' . PHP_EOL;
echo print_r($result, true) . PHP_EOL;
echo 'Finish array exporting' . PHP_EOL;

echo 'Start array (author aggregated) exporting...' . PHP_EOL;
$exporter = new Exporter();
$result = $exporter->export(new ArrayAuthorAggregatedVisitorFactory(), $a1, $a2, $a3, $a4, $a5, $a6);
echo 'Results: ' . PHP_EOL;
echo print_r($result, true) . PHP_EOL;
echo 'Finish array exporting' . PHP_EOL;

echo 'Start json (author aggregated) exporting...' . PHP_EOL;
$exporter = new Exporter();
$result = $exporter->export(new JsonAuthorAggregatedVisitorFactory(), $a1, $a2, $a3, $a5, $a6);
echo 'Results: ' . PHP_EOL;
echo print_r($result, true) . PHP_EOL;
echo 'Finish array exporting' . PHP_EOL;

Here's the output

Looking at this example is easy to understand that splitting Article into separated and more focused classes does not lead to harder handling. Of course there are more classes but that's the price for flexibility and pattern usage: the more you're abstract, flexible and have "quick to change" code, the more classes and abstraction you have in your codebase.

What's next?

Following this solution could force a change int the way you code (and rightfully so, sometimes). One of the possible scenarios left out is about persistence. When there's only an Article class persistence is just straightforward, but as soon as you model domain this way some decisions must be taken. For instance, how to handle the ids? As a transition from one state to the other happens to create a new entity, should the new entity take the old id or should it have a brand new one? If old id is used, who have to take care of persistence order (as id must be unique)? If new id is used, how to manage something like permalink or caching strategy or any kind of comparable issues? I would not provide specific answers as you might apply different choices based on different situations but, to give some pointers, if old ids are chosen, so if the id will be assigned from starting entity to new one, a solution could be to wrap persistence operations in a transaction, perform a delete first then add new entity. If new ids are chosen is it possible to use "surrogate identity" (or something similar) and provide to the outside world an immutable id. The key concept here is don't let the application or infrastructure fool you when modeling your domain: if you choose not to split things because of persistence tools like ORMs you can fall into worst issues. Moreover data is your ultimate source of truth as application and code may vary along the time but data must adhere to business/domain rules and having a unique Article entity could lead to bugs and consequentially corrupted data.

Conclusions

Status flags, sometimes, are a smell for a unique class which incapsulates much more than a single concept. When internal data is meaningful for a subset or worst for a single status you'll obtain a not so cohesive class. This kind of situation calls for a split. Also when clients behave differently depending on flags this is true. You've seen a split in action and what it entails. You're now also aware that more "uncommon" decision must be taken when dealing with splitted code (when not operating into RAD logics that's pretty much always true) and more abstract code could be part of the codebase. Be also aware that persistence have to be managed "by hand". However what you get in return is priceless: more expressive and flexible code, no null checks due to "polymorphic" nature of the class, no fuzzy exceptions, less buggy code and data.

What do you think about this solution? Is it something you've already adopted? Will you try to embrace it in your next or current project? Let us know and happy coding!