Ola peepz,

Nog even een vraagje over eigen exception classes.

Mij werd dus aangeraden om eigen exception classes te gebruiken. Nu zien die classes er allemaal ongeveer zo uit:

<?php

namespace Foo;

use Bar\MyException;

class Exception extends MyException {

public function __construct($message, $code, $previous = null) {
parent::__construct($message, $code, $previous);
}

}
?>
Nu is het zo dat de MyException class een constructor heeft die identiek is aan de constructor van de Foo Exception class. De constructor in de Foo Exception class levert op dit moment dus geen meerwaarde en zou ik dus compleet achterwege kunnen laten. (Hierbij ga ik er nu even vanuit dat ik de $message niet wil aanpassen met een of andere standaardtekst.)

Als ik de constructor zou weglaten, krijg ik als het ware een lege class. Is dat niet vreemd? Of is dit gewoon oké?

<?php

namespace Foo;

use Bar\MyException;

class Exception extends MyException {}

?>
Nee, dat is niet vreemd. Dat is zlefs zzeer normaal... overigens lijkt je code nu niet helemaal te kloppen...
Haha, Woutertje nog laat wakker :)

>> Nee, dat is niet vreemd. Dat is zlefs zzeer normaal...
Ah oke... dus een lege class is normaal :D

[offtopic]
>> overigens lijkt je code nu niet helemaal te kloppen...

Sorry... ik heb een paar biertjes teveel op... wat klopt er volgens jou niet?
[/offtopic]
Je zou wat aan de naamgeving kunnen doen. Dit is onlogisch:

class Exception extends MyException() {}

Er is al een standaardklasse Exception, dus als je van "generiek" naar "specifiek" werkt, is het eerder:

class MyException() extends \Exception() {}

Anders moet je raden of je Exception gebruikt uit de namespace of \Exception van PHP.

Tweede punt: van lege klassen die een kloon zijn van andere klassen, ben ik geen voorstander. Ja, ze zijn prachtig voor @todo-lijsten. Maar het zijn ook luchtkastelen. En lege koffers die je blijft meezeulen omdat je "ooit" bedacht hebt dat ze "later misschien" van pas komen.
Lege klassen zijn niet goed, behalve bij exception klassen. De exceptions wil je namelijk kunnen onderscheiden van elkaar, doormiddel van hun klasse. De exceptions hebben echter allemaal vaak alleen de functies die de base exception class ook heeft.
Wouter J op 16/11/2013 19:19:14

Lege klassen zijn niet goed, behalve bij exception klassen. De exceptions wil je namelijk kunnen onderscheiden van elkaar, doormiddel van hun klasse. De exceptions hebben echter allemaal vaak alleen de functies die de base exception class ook heeft.

Dat ben ik deels met je eens, maar daarmee kies je binnen de exception-hiërarchie automatisch voor de specifieke plaats van een exception in het grotere geheel. Bijvoorbeeld:

<?php
class ServiceException extends \Exception {}
class DatabaseException extends ServiceException {}
?>

Die opzet bevat al impliciet een expliciete keuze: de databasehandler is een service. Je kúnt die keuze inderdaad maken, maar besef dan dat het een ontwerpbeslissing is die consequenties voor de rest heeft. En voor later.

Via de exceptions doe je dus eigenlijk aan class type hinting. Alleen komt dat niet tot uitdrukking in de klassen zelf, waar je dat verwacht, maar alleen in de exceptions.

Als je andere exceptions slechts gebruikt om "ze van elkaar te kunnen onderscheiden", krijg je trouwens nog iets dat je sowieso niet wilt: dan heeft elke klasse een eigen exception-klasse. Dan heeft class Foo een FooException en class Bar een BarException. Het kán, maar de vraag is of je dat moet willen.
>> Lege klassen zijn niet goed, behalve bij exception klassen. De exceptions wil je namelijk kunnen onderscheiden van elkaar, doormiddel van hun klasse. De exceptions hebben echter allemaal vaak alleen de functies die de base exception class ook heeft.

Dit is inderdaad hoe ik het nu ook heb. De exceptions zijn allemaal hetzelfde, maar je wilt ze inderdaad kunnen onderscheiden zodat je ze in een catch-blok afzonderlijk kunt opvangen en afhandelen. Gevolg is dat je dan lege classes krijgt, maar als dat bij Exceptions gebruikelijk is dan zal ik het maar zo laten.

>> Als je andere exceptions slechts gebruikt om "ze van elkaar te kunnen onderscheiden", krijg je trouwens nog iets dat je sowieso niet wilt: dan heeft elke klasse een eigen exception-klasse.

Dit is wel wat ik nu in feite doe.

>> Je zou wat aan de naamgeving kunnen doen. Dit is onlogisch:

class Exception extends MyException() {}

Euh, niet helemaal. Die MyException moet je zien als mijn eigen base Exception. En als je goed kijkt dan zie je dat de namespace Foo is. Het is dus niet een normale Exception, maar een FooException. Eigenlijk is het dus dit:

class FooException extends BaseException() {}


Wouter J op 16/11/2013 01:53:05

Nee, dat is niet vreemd. Dat is zlefs zzeer normaal... overigens lijkt je code nu niet helemaal te kloppen...

Wat klopt er niet?
Je kunt inderdaad dit onderscheid maken:


<?php
class DatabaseException extends \Exception() {}
class ConnectionException extends DatabaseException() {}
?>

Dan kun je in een klasse dit doen:

<?php
if ($this->connect_error) {
    throw new \ConnectionException();
}
?>

Fijn, nu weet je dat er een ConnectionException optreedt. En ook dat dit een database-exception moet zijn, en geen andere verbindingsfout, want zo hadden we de exception-hiërarchie gedefinieerd.

Alleen... voor het type fout heb je helemaal geen apart type exception nodig. Het kan ook zo:

<?php
if ($this->connect_error) {
    throw new \Exception(
        'Connect error (' . $this->connect_errno . '): ' . $this->connect_error,
        $this->connect_errno
    );
}
?>

Met deze standaard-exception zie je toch veel meer: benut de mogelijkheid om foutmeldingen én foutcodes door te geven.

Tweede punt is dat de rek er al gauw uit is. Stel dat "Too many connections" de meest voorkomende verbindingsfout is. Dan zou je de hiërarchie verder kunnen extenden:

<?php
class DatabaseException extends \Exception() {}
class ConnectionException extends DatabaseException() {}
class TooManyConnectionsException extends ConnectionException() {}
?>

Als je weet wat er allemaal fout kan gaan, is duidelijk dat het onzinnig is om honderden exceptions te maken. Gebruik dan een duidelijke melding met een unieke code.
Ward van der Put op 17/11/2013 09:13:21


<?php
if ($this->connect_error) {
    throw new \ConnectionException();
}
?>

Fijn, nu weet je dat er een ConnectionException optreedt. En ook dat dit een database-exception moet zijn, en geen andere verbindingsfout, want zo hadden we de exception-hiërarchie gedefinieerd.

Alleen... voor het type fout heb je helemaal geen apart type exception nodig. Het kan ook zo:

<?php
if ($this->connect_error) {
    throw new \Exception(
        'Connect error (' . $this->connect_errno . '): ' . $this->connect_error,
        $this->connect_errno
    );
}
?>

Met deze standaard-exception zie je toch veel meer: benut de mogelijkheid om foutmeldingen én foutcodes door te geven.

In log files wel ja. Maar nu zou ik met stripos() aan de gang moeten om iets alleen uit te voeren bij een connectie fout.

Ward van der Put op 17/11/2013 09:13:21

Tweede punt is dat de rek er al gauw uit is. Stel dat "Too many connections" de meest voorkomende verbindingsfout is. Dan zou je de hiërarchie verder kunnen extenden:

<?php
class DatabaseException extends \Exception() {}
class ConnectionException extends DatabaseException() {}
class TooManyConnectionsException extends ConnectionException() {}
?>

Als je weet wat er allemaal fout kan gaan, is duidelijk dat het onzinnig is om honderden exceptions te maken. Gebruik dan een duidelijke melding met een unieke code.

Je moet het inderdaad niet gaan overdrijven. Een combinatie van error nummers met een exception class die specifiek genoeg is om te kunnen bepalen wat het nummer betekend is vaak toch wel de lijn die je niet voorbij wilt gaan.
Ward, nummers kun je niet gebruiken om alleen sommige exceptions te catchen. Wanneer je dus onderscheidt wil maken bij het catchen gebruik je een andere exception klasse. In jou geval zou ik bijv. onderscheid maken tussen database connection exceptions en database no results found exception. Echter binnen de connection exceptions wil je het verschil niet apart catchen. Ik ga niet alleen de TooManyConnections exception catchen, maar niet de NoAccess exception. Dat onderscheid bouw je dus in met exception nummers (waar natuurlijk een mooie class constante aan hangt) en messages.
Mijn idee is om niet om dit te doen:

<?php
class DatabaseException extends \Exception() {}
class ConnectionException extends DatabaseException() {}
?>
Maar eerder zoiets als dit:

<?php
class DatabaseException extends MyBaseException {}
class DatabaseConnectionException extends MyBaseException {}
?>
Ik breng dus juist geen hiërarchie aan. Nu staat iedere exception op zichzelf. Op het moment dat een exception wordt gegooid kan ik deze dus "per stuk" afvangen en een eigen afhandeling geven.

Stel dat ik bijv. dit zou doen:

<?php
class ConnectionException extends DatabaseException() {}
class NoDataException extends DatabaseException() {}
?>
... dan zou als ik ergens een DatabaseException opvang, de afhandeling voor een niet werkende connectie (situatie 1) en het niet verkrijgen van data (situatie 2) hetzelfde worden afgehandeld. Situatie 2 kan echter in de praktijk gewoon voorkomen zonder dat dit een ernstige fout hoeft te zijn. Situatie 1 is echter een ander geval. Als ik geen connectie met de database kan maken moeten alle alarmbellen afgaan en moet ik actie ondernemen.

Is mijn gedachtengang correct?


Alleen... voor het type fout heb je helemaal geen apart type exception nodig. Het kan ook zo:

<?php
if ($this->connect_error) {
throw new \Exception(
'Connect error (' . $this->connect_errno . '): ' . $this->connect_error,
$this->connect_errno
);
}
?>

Maar Ward, dan ondermijn je denk ik het principe van Exceptions. De Exception class zelf weet nu, aan de hand van de $message, om wat voor error het gaat. Echter, de buitenwereld heeft geen idee. Op deze manier kun je alleen een algemene exception opvangen in een catch-blok, die altijd op dezelfde manier wordt afgehandeld. Maar wat je wil is toch dat FooException op manier A wordt afgehandeld, en BarExceptopn op manier B, enz. Door slechts 1 Exception class te gebruiken haal je dit principe geheel onderuit.

Reageren