0

I have to send invoices to cutomers through the related customer's API.
Each customer chooses how to be contacted (email, fax, sms...).
My problem is that each time I add a new "contact type" I increase the cyclomatic complexity of my script.
Here is my code :
<?php

interface ExternalApi
{
    public function sendByEmail();
    public function sendByFax();
    public function sendBySMS();
    // more sending modes ...
}

class Api1 implements ExternalApi
{   
    public function sendByEmail()
    {
        echo __CLASS__." sending email<br>\n";
    }
    
    public function sendByFax()
    {
        echo __CLASS__." sending fax<br>\n";
    }
    
    public function sendBySMS()
    {
        echo __CLASS__." sending SMS<br>\n";
    }
}

class Customer
{
    public const EMAIL = 'email';
    public const FAX = 'fax';
    public const SMS = 'sms';
    public const EMAIL_AND_FAX = 'email_and_fax';
    public const PHONE = 'phone';
    public const PLANE = 'plane';
    public const BOAT = 'boat';
    public const SATELITE = 'satelite';
    public const CAB = 'cab';
    // more contact types...
    
    public ?string $contactType;
}

class Invoice
{
    public Customer $customer;
    
    public function __construct(Customer $customer)
    {
        $this->customer = $customer;
    }
}

class InvoiceSender
{
    private ExternalApi $api;

    public function __construct(ExternalApi $api)
    {
        $this->api = $api;
    }
    
    public function send(Invoice $invoice)
    {
        switch($invoice->customer->contactType) {
            case Customer::EMAIL :
                $this->api->sendByEmail();
                break;
            case Customer::FAX :
                $this->api->sendByFax();
                break;
            case Customer::SMS :
                $this->api->sendBySMS();
                break;
            case Customer::EMAIL_AND_FAX:
                $this->api->sendByEmail();
                $this->api->sendByFax();
                break;
            // more cases ...
        }
    }
}

$customer = new Customer();
$customer->contactType = Customer::EMAIL_AND_FAX;
$invoice = new Invoice($customer);
$api = new Api1();
$invoiceSender = new InvoiceSender($api);
$invoiceSender->send($invoice);

As you can see the switch statement of InvoiceSender::send is increased each time I add a new "contact type".
Do you know which design pattern could solve this problem ?

Shaolin
  • 415
  • 5
  • 11
  • 1
    Extract these as separate classes into an associative array, then fetch from there by `$invoice->customer->contactType`, and execute a `run()` method or something common between them. Classic [strategy pattern](https://en.wikipedia.org/wiki/Strategy_pattern), which happens to help out with the cyclomatic complexity here. – VLAZ Dec 26 '20 at 23:22

2 Answers2

1

Create an interface like SendInvoiceInterface with a send and a shouldSend method. Then create 3 implementations:

  • EmailInvoiceSender
  • FaxInvoiceSender
  • SMSInvoiceSender

Next inject these all into your InvoiceSender as an array. Then instead of the switch-case you can loop over all the senders and 'ask' them if they shouldSend. If that evaluates to true you can call send on them.

This way the logic stays inside each sender itself instead of the growing switch-case.

PtrTon
  • 3,705
  • 2
  • 14
  • 24
0

Thank you guys for your advices.
Your suggestion about using an array with all the services inside brings me to another solution :

<?php

interface ExternalApi
{
    public function getSendingModes();
}

class Api1 implements ExternalApi
{   
    public function getSendingModes()
    {
        return [
            Customer::EMAIL => fn() => $this->sendByEmail(),
            Customer::FAX => fn() => $this->sendByFax(),
            Customer::SMS => fn() => $this->sendBySMS(),
            Customer::EMAIL_AND_FAX => function() {
                $this->sendByEmail();
                $this->sendByFax();
            },
        ];
    }
    
    private function sendByEmail()
    {
        echo __CLASS__." sending email<br>\n";
    }
    
    private function sendByFax()
    {
        echo __CLASS__." sending fax<br>\n";
    }
    
    private function sendBySMS()
    {
        echo __CLASS__." sending SMS<br>\n";
    }
}

class Customer
{
    public const EMAIL = 'email';
    public const FAX = 'fax';
    public const SMS = 'sms';
    public const EMAIL_AND_FAX = 'email_and_fax';
    public const PHONE = 'phone';
    public const PLANE = 'plane';
    public const BOAT = 'boat';
    public const SATELITE = 'satelite';
    public const CAB = 'cab';
    // more contact types...
    
    public ?string $contactType;
}

class Invoice
{
    public Customer $customer;
    
    public function __construct(Customer $customer)
    {
        $this->customer = $customer;
    }
}

class InvoiceSender
{
    private ExternalApi $api;
    private array $sendingModes;

    public function __construct(ExternalApi $api)
    {
        $this->api = $api;
        $this->sendingModes = $api->getSendingModes();
    }
    
    public function send(Invoice $invoice)
    {
        $sendingMode = $this->sendingModes[$invoice->customer->contactType] ?? null;
        if (null === $sendingMode) {
            throw new RuntimeException("API ".get_class($this->api)." doesn't have sending mode ".$invoice->customer->contactType);
        }
        $sendingMode();
    }
}

$customer = new Customer();
$customer->contactType = Customer::EMAIL_AND_FAX;
//$customer->contactType = Customer::EMAIL;
//$customer->contactType = Customer::BOAT;
$invoice = new Invoice($customer);
$api = new Api1();
try {
    $invoiceSender = new InvoiceSender($api);
    $invoiceSender->send($invoice);
} catch (Exception $ex) {
    echo $ex->getMessage()."\n";
}

I change the ExternalApi interface in order to retreive an array of sending modes.
Doing so, each API is aware of its own sending modes and so each API don't need to implement sending mode that they don't handle.
Each value of the array is an anonymous function which trigger the private methods of the API class.
Also I don't have to write extra classes for each sending modes.

Shaolin
  • 415
  • 5
  • 11