exception classes
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:
Code (php)
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é?
Code (php)
1
2
3
4
5
6
7
8
9
2
3
4
5
6
7
8
9
<?php
namespace Foo;
use Bar\MyException;
class Exception extends MyException {}
?>
namespace Foo;
use Bar\MyException;
class Exception extends MyException {}
?>
Gewijzigd op 16/11/2013 01:37:32 door Ozzie PHP
>> 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?
Gewijzigd op 16/11/2013 02:50:50 door Ozzie PHP
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.
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:
Code (php)
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.
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?
Code (php)
Dan kun je in een klasse dit doen:
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:
Code (php)
1
2
3
4
5
6
7
8
2
3
4
5
6
7
8
<?php
if ($this->connect_error) {
throw new \Exception(
'Connect error (' . $this->connect_errno . '): ' . $this->connect_error,
$this->connect_errno
);
}
?>
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:
Code (php)
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.
Gewijzigd op 17/11/2013 09:14:45 door Ward van der Put
Ward van der Put op 17/11/2013 09:13:21:
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:
Met deze standaard-exception zie je toch veel meer: benut de mogelijkheid om foutmeldingen én foutcodes door te geven.
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:
Code (php)
1
2
3
4
5
6
7
8
2
3
4
5
6
7
8
<?php
if ($this->connect_error) {
throw new \Exception(
'Connect error (' . $this->connect_errno . '): ' . $this->connect_error,
$this->connect_errno
);
}
?>
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:
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.
Code (php)
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.
Code (php)
Maar eerder zoiets als dit:
Code (php)
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:
Code (php)
... 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?
Quote:
Alleen... voor het type fout heb je helemaal geen apart type exception nodig. Het kan ook zo:
Code (php)
1
2
3
4
5
6
7
8
2
3
4
5
6
7
8
<?php
if ($this->connect_error) {
throw new \Exception(
'Connect error (' . $this->connect_errno . '): ' . $this->connect_error,
$this->connect_errno
);
}
?>
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.
Gewijzigd op 17/11/2013 14:16:43 door Ozzie PHP
En dat is juist het mooie van het gebruik van hierarchy. In sommige gevallen wil je gewoon alle database problemen, of zelfs alle problemen, op vangen, terwijl je in andere gevallen alleen de connectie problemen wil opvangen. Het gebruik van hierarchy is juist het mooie van exceptions, gooi je dat weg kan je lekker alleen met een base klasse gaan werken...
En voor die laatste alinea, lees eens mijn reactie. Overal een exception klasse voor maken is onzin (vooral als je geen hierarcy gebruikt,..)
Een class Foo voorzien van een FooException, een class Bar van een BarException, enzovoort kán inderdaad een oplossing zijn voor klasse-specifieke fouten. Maar er zijn ook universele fouten die niet klasse-gebonden zijn. Neem bijvoorbeeld een fout van het type "ongeldig datatype" of een fout uit de categorie "waarde te groot/klein".
Een voorbeeld... iDEAL vereist een bedrag in centen (een integer) tussen 84 en 1000000. Toch vind ik het opgeven van iets anders dan een integer geen iDEAL-specifieke iDEALException waard. Het is een veel universelere fout: deze methode verwacht een integer, maar er kwam iets anders binnen.
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
<?php
/**
* @api
* @param int $amount An iDEAL amount in cents.
* @return this
* @throws \InvalidArgumentException if the amount is not an integer.
* @throws \RangeException if the amount is too small or too large.
*/
public function setAmount($amount)
{
$amount = filter_var($amount, FILTER_VALIDATE_INT);
if ($amount === false) {
throw new \InvalidArgumentException('The iDEAL amount is not an integer.');
} elseif ($amount < 84) {
throw new \RangeException('The iDEAL amount is too small.');
} elseif ($amount > 1000000) {
throw new \RangeException('The iDEAL amount is too large.');
} else {
$this->Amount = $amount;
}
return $this;
}
?>
/**
* @api
* @param int $amount An iDEAL amount in cents.
* @return this
* @throws \InvalidArgumentException if the amount is not an integer.
* @throws \RangeException if the amount is too small or too large.
*/
public function setAmount($amount)
{
$amount = filter_var($amount, FILTER_VALIDATE_INT);
if ($amount === false) {
throw new \InvalidArgumentException('The iDEAL amount is not an integer.');
} elseif ($amount < 84) {
throw new \RangeException('The iDEAL amount is too small.');
} elseif ($amount > 1000000) {
throw new \RangeException('The iDEAL amount is too large.');
} else {
$this->Amount = $amount;
}
return $this;
}
?>
Stel ik heb bijv. deze situatie:
class A probeert iets te cachen en gebruikt hiervoor de cacher (class C). Deze cacher gebruikt op haar beurt weer een FileSystem class (class F) om het bestand op te slaan. Dus:
class A -> class C -> class F
Als een bestand niet kan worden weggeschreven gooit class F een FileSystem exception. Vervolgens gooit class C een Cacher exception. Class A controleert vervolgens in het catch-blok of er een Cacher exception is gegooid. Kan ik nu dan ook net zo goed in plaats van een FileSystem exception en een Cacher exception gewoon 2x een algemene exception gooien? En in class A controleren of er een algemene Exception is gegooid?
Wouter J op 17/11/2013 15:01:30:
Ward, in dat geval zal ik alsnog een Ideal\RangeException en Ideal\InvalidArgumentException maken.
Betaalmethoden extenden hetzelfde request-object voor de API van een betaalprovider en hebben allemaal een setAmount()-methode. Ik heb daarmee zoiets:
Code (php)
Waar zou jij de grens voor wel/geen unieke exception dan leggen? Bij de namespace? De Request-ouderklasse? Of toch de iDEAL-klasse? Of misschien alles een kwartslag draaien en een InvalidAmountException opvoeren voor alle betaalsystemen?
Gewijzigd op 17/11/2013 15:20:09 door Ward van der Put
Ozzie PHP op 17/11/2013 15:30:37:
Ward, kun je ook nog ff reageren op mijn vraag...
Ja, ik wil hier een CacherException zien. Dat de Cacher het FileSystem gebruikt, is niet relevant. Een verbeterde Cacher 2.0.0 kan een andere store gebruiken: het RAM-geheugen, een database, de cloud. Maar daarvan mogen "derden" die de Cacher inzetten geen last hebben. Ze hoeven het niet eens te weten.
Overigens kun je uit een trace altijd nog afleiden waar die CacherException vandaan komt als je gaat debuggen.
Ik zou inderdaad niet per klasse is specifieks doen, behalve als het specifiek is voor die betaalmethode. Globaal kun je je code altijd indelen in verschillende componenten: Router, Validation, Form, etc. Deze component hebben allemaal hun eigen exceptions.
Ik zou dus een TargetPay\Exception\InvalidAmountException maken. Over het algemeen moeten alle children van een klasse dezelfde exceptions gebruiken, aangezien je ongemerkt de klasse moet kunnen veranderen.
Toevoeging op 17/11/2013 15:40:07:
>> Vervolgens gooit class C een Cacher exception. Class A controleert vervolgens in het catch-blok of er een Cacher exception is gegooid. Kan ik nu dan ook net zo goed in plaats van een FileSystem exception en een Cacher exception gewoon 2x een algemene exception gooien? En in class A controleren of er een algemene Exception is gegooid?
Nee, geen algemene exceptions. De Class A zal echter alleen een Cacher exception willen ontvangen (met als previous exception de FileSystemException). Het zal me een worst wezen wat die cacher heeft gedaan waardoor het niet lukte, dat heb ik alleen nodig bij het debuggen. Voor de code wil ik alleen weten dat die Cacher het niet is gelukt.
Dus mijn opzet zoals ik die nu heb die klopt dan gewoon?