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 {}

?>
Ja, message en code komt op het zelfde neer. De code is echter voor de computer goed begrijpbaar en de message voor de mens.
Kijk maar eens naar http://kohanaframework.org/3.3/guide-api/HTTP_Exception dan.

In Request_Client_Internal::execute_request() staat het volgende:
<?php

try
{
...
}
catch (HTTP_Exception $e)
{
// Get the response via the Exception
$response = $e->get_response();
}
catch (Exception $e)
{
// Generate an appropriate Response object
$response = Kohana_Exception::_handler($e);
}

?>

Op die manier kun je de get_response() van HTTP_401_Exception en HTTP_404_Exception overschrijven en de rest (op HTTP_Expected en subclasses na) houd het standaard gedrag.

Zonder een hiërarchie zou je heel veel code herhalen. Wat niet bepaald DRY is.
>> Ja, message en code komt op het zelfde neer. De code is echter voor de computer goed begrijpbaar en de message voor de mens.

Oke, duidelijk. Maar wat doe je dan met die code? Waarvoor gebruik je die?
Ik ben even de weg kwijt.

Als je bijv. een filesystem class hebt, wat voor soorten exceptions moet die dan kunnen gooien? Ik dacht in 1e instantie dat deze class gewoon telkens als er iets fout gaat een FilesystemException zou moeten gooien. Eerder werd ergens gesteld dat iedere class z'n eigen exception moet hebben, dus dit leek me de juiste oplossing. Nu loop ik tegen het probleem aan dat ik in de problemen kom.

Stel ik doe in class Foo dit:

<?php

try {
$this->filesystem->deleteDirectory($dir);
} catch (FilesystemException $e) {
// directory kan niet worden verwijderd
}

?>
En in de Filesystem class staat zoiets:

<?php

public function deleteDirectory($dir) {
if (is_dir($dir)) {
throw new FilesystemException('dir already exists');
} else {
if (!rmdir($dir)) {
throw new FilesystemException('could not remove dir');
}
}
}

?>
Zoals je ziet kunnen er in deze method 2 exceptions optreden. Op het moment dat de directory al bestaat, en op het moment dat de directory niet kan worden verwijderd.

We kunnen nu geen onderscheid maken tussen de beide exceptions. Oplossing: we gaan 2 verschillende exceptions gooien en vangen:

Class Foo:

<?php

try {
$this->filesystem->deleteDirectory($dir);
} catch (RunTimeException $e) {
// directory kan niet worden verwijderd
}

?>
En de Filesystem class:

<?php

public function deleteDirectory($dir) {
if (is_dir($dir)) {
throw new UnexpectedValueException('dir already exists');
} else {
if (!rmdir($dir)) {
throw new RunTimeException('could not remove dir');
}
}
}

?>
Nu gaat het wel goed, maar gebruik ik ineens dus helemaal geen FilesystemException meer. Dat lijkt me ook niet kloppen toch? Ik begreep eerder dat het goed is dat een class zijn eigen exception heeft, maar als ik in dit voorbeeld een FilesystemException zou gebruiken, dan werkt het niet. Dus ik snap nu even niet A) of je wel of niet eigen exceptions moet gebruiken en B) zo ja, wanneer en op welke manier. Ik zit aardig op het goede spoor, maar iets klopt er nog niet helemaal in mijn gedachten. Kan iemand me weer wat wijzer maken?
[quote="Ozzie PHP op 01/12/2013 03:29:42"]
<?php
public function deleteDirectory($dir) {
if (is_dir($dir)) {
throw new UnexpectedValueException('dir already exists');
} else {
if (!rmdir($dir)) {
throw new RunTimeException('could not remove dir');
}
}
}
?>
[/code]
Je maakt een logicafout. Je kunt alleen een directory verwijderen die bestaat. Je zou hier dus !is_dir($dir) voor "is geen directory" verwachten. Daarmee krijg je één FilesystemException voor twee samenhangende fouten:

<?php
/**
 * @param string $dir Path to the directory to delete.
 * @return bool
 * @throws FilesystemException
 */
public function deleteDirectory($dir)
{
    if (!is_dir($dir) || !rmdir($dir)) {
        throw new FilesystemException('Could not delete directory.');
    }
    return true;
}
?>

Als je het even test, blijkt rmdir() zelf al te controleren of de directory bestaat. Bestaat de directory namelijk niet, dan krijg je een warning: "No such file or directory." Aangezien een is_dir() al is geïmplementeerd in rmdir(), kun je het vereenvoudigen tot:

<?php
/**
 * @param string $dir Path to the directory to delete.
 * @return bool
 * @throws FilesystemException
 */
public function deleteDirectory($dir)
{
    if (!rmdir($dir)) {
        throw new FilesystemException('Could not delete directory.');
    }
    return true;
}
?>
Jij zit goed op te letten Ward. Dat krijg je dus als je veel te laat een bericht post. Wat jij zegt klopt, maar er zit een kleine kanttekening aan.

Als je een directory verwijdert die niet bestaat en je gooit vervolgens een FilesystemException, dan zal class Foo dus denken dat het verwijderen is mislukt, en dat de directory nog steeds bestaat. Class Foo zal dus denken dat het fout gaat.

Een beter en duidelijker voorbeeld is het aanmaken van een directory. Zelfde principe... Class Foo maakt een directory aan en wil daarna een aantal bestanden in die directory plaatsen. Jouw code (aangepast):

<?php
/**
* @param string $dir Path to the directory to create.
* @return bool
* @throws FilesystemException
*/
public function createDirectory($dir)
{
if (!mkdir($dir)) {
throw new FilesystemException('Could not delete directory.');
}
return true;
}
?>
En in class Foo:

<?php

try {
$this->filesystem->createDirectory($dir);
} catch (FilesystemException $e) {
// directory kan niet worden aangemaakt
}

?>
Stel nu dat de directory al bestaat, dan geeft jouw code een FilesystemException, waardoor class Foo zal denken dat de directory niet kon worden aangemaakt. Class Foo gaat er dus vanuit dat de directory niet bestaat en zal er dus ook geen bestanden in gaan plaatsen. Het gaat dan dus mis, terwijl de bestanden eigenlijk wel hadden kunnen worden geplaatst, omdat de directory al bestond.

Het gooien van slechts 1 type exception is dus volgens mij niet toereikend???
Ozzie PHP op 01/12/2013 10:52:19

Stel nu dat de directory al bestaat, dan geeft jouw code een FilesystemException, waardoor class Foo zal denken dat de directory niet kon worden aangemaakt. Class Foo gaat er dus vanuit dat de directory niet bestaat en zal er dus ook geen bestanden in gaan plaatsen. Het gaat dan dus mis, terwijl de bestanden eigenlijk wel hadden kunnen worden geplaatst, omdat de directory al bestond.

De klasse Foo denkt zelf niets, maar belandt in de catch. Daarin plaats je vervolgens een oplossing.

Je voorbeeld bevat al een duidelijke ontwerpbeslissing: het maken van een directory mag mislukken als de directory al bestaat, want dan kunnen we daarin bestanden opslaan. Dan kun je dat formaliseren in de naam en code van de methode. Even in een complete if-elseif-else om alle cases af te dekken:

<?php
/**
 * @param string $dir Path to the directory to check for and create.
 * @return bool
 * @throws FilesystemException
 */
public function createDirectoryIfNotExists($dir)
{
    if (is_dir($dir)) {
        return true;
    } elseif (!mkdir($dir)) {
        throw new FilesystemException('Could not create directory.');
    } else {
        return true;
    }
}
?>

Gaat dat je te ver, dan moet je de try of de catch van Foo uitbreiden. En dat moet eigenlijk ook als je ontwerpbeslissing vooral voor Foo geldt.
Thanks Ward. Dat vind ik dus het lastige... want als de directory inderdaad al bestaat, zoals jij zegt, is er dan wel of niet sprake van een exception? Is het aanmaken van een directory die al blijkt te bestaan wel of niet een exception?

Het valt me trouwens op dat je true returnt. Is dat iets wat je altijd doet? Ik bedoel, waarom return je een boolean als je toch al een exception gooit? Dat lijkt me dubbelop?
Je kunt inderdaad ook null retourneren: dan heb je null voor "nothing to do" en true voor "directory created". En eigenlijk hadden we hier binnen class Filesystem een uitstapje moeten maken naar Filesystem::directoryExists(). Maar het gaat even om de gedachte.

Als jij het okay vindt dat een directory niet wordt gemaakt als die al bestaat, dan is dat geen exception waard. Het systeem gedraagt zich dan namelijk zoals jij het ontworpen hebt.
>> Je kunt inderdaad ook null retourneren: dan heb je null voor "nothing to do" en true voor "directory created".

Waarom return je per se iets? Je kunt toch ook helemaal niks returnen, maar alleen een exception gooien? Als er dan geen exception wordt opgevangen weet je dat het goed is gegaan :) Het heeft (wat mij betreft) alleen zin om iets te returnen als je ook iets met die waarde gaat doen, maar dat is nu niet het geval.

Filesystem::directoryExists() ... haha, ja daar heb je dus een punt waar ik lang over heb moeten denken. Maar dan zou ik dus altijd voordat ik een directory aanmaak, moeten controleren of die wel of niet bestaat. Ik weet niet of dat handig is? Doe jij dat ook op die manier?

Reageren