[php] Overkoepelende Exception class

[Saven]

Hi all,

Het zit als volgt. Ik gebruik een aantal externe libraries/classes. Die gooien allemaal hun eigen Exception. Niet relevant, maar bijvoorbeeld: Swift_SMTP_Exception of PHPMailer_Exception.

Ik extend deze libraries met een eigen wrapper, zodat ik altijd dezelfde functienamen kan aanroepen ongeacht welke library gebruikt wordt. Voorbeeld met dummy-/pseudecode

PHP:

namespace Clients\Swift;

class MyMailer extends Swift_Mailer
{
    public static function instance(Company $company)
    {
        if (self::$instance)
        {
            return self::$instance
        }

        return self::$instance = new self($company->smtp_server, $company->smtp_password);
     }

     public function send($to, $content)
     {
          $transport = $this->transport();
          $mailer = $transport->to($to)->content($content);

          return $mailer->send(); //Kan exception Swift_Mailer_Exception gooien
     }
}

PHP:

namespace Clients\Phpmailer;

class MyMailer extends PHP_Mailer
{
    public static function instance(Company $company)
    {
        if (self::$instance)
        {
            return self::$instance
        }

        $instance = new self('blabla');
        
        return self::$instance = $instance->setServer($company->smtp_server)->setPassword($company->smtp_password);
     }

     public function send($to, $content)
     {
          $transport = $this->initiate();
          $$mailer = $transport->instance();
          $mailer->setTo($to)
          $mailer->setContent($content);

          return $mailer->sendEmail(); //Kan exception PHP_Mailer_Exception gooien
     }
}

Misschien ben ik een mongool met deze opzet Dan hoor ik het ook graag
Maar in principe zijn er twee aanvliegroutes.

1. Alle exceptions handmatig afvangen. Veel herhaalwerk en niet schaalbaar:

PHP:

try
{
   $mailer->send()
}
catch( Swift_Mailer_Exception $e )
{

}
catch( PHP_Mailer_Exception $e )
{

}

Los van het feit dat het er nu twee zijn, maar later wellicht nog meer.

Een andere optie is imo gewoon een catch( Exception $e ). Maar dat lijkt me meer de dirty workaround.

Idealiter probeer ik zoiets te bereiken.

PHP:

$mailClient = $company->getMyMailerClient(); // haalt automatisch de juiste MyMailer class op.

try
{
     $sendMail = $mailClient->send('[e-mail]', 'My Awesome Email Content');
}
catch( MyCustomMailerException $e )
{
     //.... ??
}

Dus in feite een 'overkoepelende' Exception MyCustomMailerException die wordt gegooid zodra er een Swift_Mailer_Exception of PHP_Mailer_Exception opspeelt.

Een andere opzet zou ook nog kunnen zijn om in elke method van elke client wrapper de Exception af te vangen en een nieuwe Exception te gooien?

PHP:

     public function send($to, $content)
     {
         try
         {
               $transport = $this->initiate();
               $$mailer = $transport->instance();
               $mailer->setTo($to)
               $mailer->setContent($content);

               return $mailer->sendEmail(); //Kan exception PHP_Mailer_Exception gooien
         }
         catch( Swift_Mailer_Exception $e)
         {
              throw new MyCustomMailerException($e->getMessage(), $e->getCode(), $e->getPrevious());
         }
     }

Maar dat lijkt me ook niet helemaal geweldig.

Iemand die hier ervaring mee heeft en de juiste richting kan wijzen?

[MueR]

Feitelijk is je laatste optie wel de enige juiste als je in je eigen code met maar 1 type exception wil dealen.

[Woy]

Sowieso is dat natuurlijk een verantwoordelijkheid van je wrapper/adapter classes. Die weten van de implementatie, en dat moet je niet naar buiten toe lekken. Een andere library werkt misschien wel weer met return codes i.p.v. exceptions.

Exceptions horen gewoon bij je contract/interface, dus je wrapper classes zullen die ook moeten converteren naar de uniforme interface.

[Saven]

MueR schreef op maandag 17 augustus 2020 @ 13:33:
Feitelijk is je laatste optie wel de enige juiste als je in je eigen code met maar 1 type exception wil dealen.

Duidelijk thnx Laatste optie it is

Woy schreef op maandag 17 augustus 2020 @ 13:38:
Sowieso is dat natuurlijk een verantwoordelijkheid van je wrapper/adapter classes. Die weten van de implementatie, en dat moet je niet naar buiten toe lekken. Een andere library werkt misschien wel weer met return codes i.p.v. exceptions.

Exceptions horen gewoon bij je contract/interface, dus je wrapper classes zullen die ook moeten converteren naar de uniforme interface.

Laatste alinea begrijp ik niet helemaal. Hoe bedoel je dat de wrapper classes 'die' ook moeten converteren naar de uniforme interface?

[DataGhost]

Saven schreef op maandag 17 augustus 2020 @ 17:25:
[...]

Laatste alinea begrijp ik niet helemaal. Hoe bedoel je dat de wrapper classes 'die' ook moeten converteren naar de uniforme interface?

Nou,

Saven schreef op maandag 17 augustus 2020 @ 13:19:
Ik extend deze libraries met een eigen wrapper, zodat ik altijd dezelfde functienamen kan aanroepen ongeacht welke library gebruikt wordt.

Vanuit alle andere code gezien praat die code alleen maar met jouw library (wrapper). Dan kan je toch niet opeens exceptions van andere libraries naar buiten laten lekken? Als jij straks besluit een derde library te ondersteunen in je wrapper zou je alle code (ook evt van andere developers) aan moeten gaan passen om de exceptions van die extra library ook te handlen, of jij regelt dat in jouw code en gooit een exception die bij jouw library hoort. Het "contract" is inderdaad de interface en die zou niet zomaar moeten veranderen naar aanleiding van wat er intern toevallig gebruikt wordt.

[Voutloos]

Je focust op de Exceptions, maar je noemt al dat je een generieke interface wil hebben. Jouw wrappers kunnen dat niet voor elkaar krijgen, want de verschillende classes die je extend kunnen methodes hebben die met elkaar botsen.

Dus ik zou eerst voor Composition over Inheritance gaan, en daarna als 1 van de details om generiek te maken naar de Exceptions kijken.

Of kort door de bocht vanuit de andere kant: De manier waarop je nu inheritance gebruikt heeft nogal vaak de neiging om pijnlijk te worden en bovendien niet te slagen in de bedoelde abstractie.

[bobdetweaker]

Eventjes mijn mening spuien:

1. Een static getInstance method is bad practise. Gebruik een factory om de instance van je class te leveren en bewaar die zodat je die kunt opvragen. Gebruik liever Dependency Injection dan Singleton Pattern.
2. Het afvangen van Exceptions alleen maar om je eigen Exception te gooien is prima, maar gebruik dan niet $exception->getPrevious() as previous, maar gewoon de gegooide exception.
3. Een interface zal niets toevoegen aan het gebruiksgemak van je library. In plaats van een interface te ontwerpen voor een reeks exceptions kan je beter één Exception class ontwerpen die op later tijdstip nog verdere extensies zou kunnen krijgen.