Search code examples
phpdesign-patternsfactoryfactory-pattern

Duplicate conditional statements in the factory


How to get rid of them? I'm wondering if there is a pattern or something that addresses this problem. Basically I need to instantiate a concrete child class based on the type property of another class, i.e. if type=1 then new A, else if type=2 then new B etc. I've ended up with this kind of a factory in the class with the type property:

/**
 * Get a ticket decorator based on the ticket type
 * @return ReferralService\TicketDecorator
 * @throws Exception
 */
public function getTicketDecorator(): ReferralService\TicketDecorator
{
    if (!$this->code) {
        throw new Exception("Couldn't create a ticket wrapper based on the type without a code");
    }

    /**
     * The debug service
     * @var Debug\Service $debugService
     */
    $debugService = app(Debug\Service::class);
    $debugService->setDebug(config('referral.debug'));

    switch ($this->code) {
        case self::TYPE_FEEDBACK:
            return new ReferralService\TicketDecorator\FeedbackTicketDecorator($debugService);
            break;
        case self::TYPE_BIRTHDAY:
            return new ReferralService\TicketDecorator\BirthdayTicketDecorator($debugService);
            break;
        case self::TYPE_NEW_PARTNER:
            return new ReferralService\TicketDecorator\PartnerTicketDecorator($debugService);
            break;
        default:
            throw new Exception(sprintf("Couldn't instantiate a ticket decorator based on the %s type", $this->code));

    }
}

/**
 * Instantiate a private page based on the ticket type
 * @param ReferralService\Service $service
 * @param Referrer $referrer
 * @param Ticket $ticket
 * @return ReferralService\Page\PrivatePage
 * @throws Exception
 */
public function getPrivatePage(ReferralService\Service $service, Referrer $referrer, Ticket $ticket): ReferralService\Page\PrivatePage
{
    if (!$this->code) {
        throw new Exception("Couldn't create a private page based on the type without a code");
    }

    switch ($this->code) {
        case self::TYPE_FEEDBACK:
            return new ReferralService\Page\PrivatePage\EmailReference($this->service, $referrer, $ticket);
            break;
        case self::TYPE_BIRTHDAY:
            return new ReferralService\Page\PrivatePage\Birthday($this->service, $referrer, $ticket);
            break;
        case self::TYPE_NEW_PARTNER:
            return new ReferralService\Page\PrivatePage\Partner($this->service, $referrer, $ticket);
            break;
        default:
            throw new Exception(sprintf("Could't find a page for the type", $this->code));
    }
}

Every method in the factory tests the type field, this looks clumsy for me. I thought to have a separate child class for every type and use factory methods without conditional statements, but I can't do so with Laravel models.


Solution

  • It's a very common refactoring pattern to replace conditionals by polymorphism.

    You could just implement a factory to create specialized TicketTypes and implement factory methods on the ticket types to create concrete TicketDecorator and PrivatePage.

    However, just keep in mind that this does introduce coupling between TicketType and the concrete classes it creates. If it preferable to avoid such coupling then stick to your initial design.