1

Ceci est écrit en PHP, mais c'est vraiment agnostique.Exceptions: Est-ce une bonne pratique?

try 
{ 
    try 
    { 
     $issue = new DM_Issue($core->db->escape_string($_GET['issue'])); 
    } 
    catch(DM_Exception $e) 
    { 
     throw new Error_Page($tpl, ERR_NOT_FOUND, $e->getMessage()); 
    } 
} 
catch(Error_Page $e) 
{ 
    die($e); 
} 

Est imbriqué essayer, attraper des blocs une bonne pratique à suivre? Il semble un peu encombrant juste pour une page d'erreur - cependant mon Datamanager émet une Exception si une erreur survient et je considère que c'est une bonne façon de détecter les erreurs.

L'exception Error_Page est simplement un compilateur de page d'erreur.

Je pourrais juste être pédant, mais pensez-vous que c'est un bon moyen de signaler les erreurs et si oui pouvez-vous suggérer une meilleure façon d'écrire cela?

Merci

+0

Oh, et changez le titre du message pour mieux refléter le sujet; "Est-ce une bonne pratique" n'est pas vraiment descriptif. Que diriez-vous de "Exceptions: Blocs try/catch imbriqués?" – Aeon

Répondre

10

Vous utilisez des exceptions pour la logique de page, et je pense personnellement que ce n'est pas une bonne chose. Des exceptions devraient être utilisées pour signaler quand des choses mauvaises ou inattendues se produisent, pas pour contrôler la sortie d'une page d'erreur. Si vous souhaitez générer une page d'erreur basée sur des exceptions, envisagez d'utiliser set_exception_handler. Toutes les exceptions non interceptées sont exécutées par la méthode de rappel que vous spécifiez. Gardez à l'esprit que cela n'arrête pas la "fatalité" d'une exception. Après une exception est passée à travers votre rappel, l'exécution s'arrêtera comme normale après toute exception non interceptée.

2

Je pense que vous feriez mieux de ne pas imbriquer. Si vous prévoyez plusieurs types d'exceptions, ayez plusieurs captures.

try{ 
    Something(); 
} 
catch(SpecificException se) 
{blah();} 
catch(AnotherException ae) 
{blah();} 
+0

Le problème avec plusieurs captures dans ce cas est que je peux attraper soit une DM_Exception OU une Error_Page - ce qui n'est pas ce que je veux faire. – Ross

+0

Si vous interceptez une exception particulière que vous souhaitez ignorer, utilisez simplement throw; (qui relance simplement l'exception) ou lance une nouvelle MyException ("Voici mon erreur", innerException) (qui crée une nouvelle exception et construit une trace de pile avec innerException). –

0

En fonction de vos besoins cela pourrait être bien, mais je suis en général assez réticents à attraper une exception, envelopper le message dans une nouvelle exception, et réémettre parce que vous perdez la trace de la pile (et éventuellement d'autres) l'information de l'exception d'origine dans l'exception d'emballage. Si vous êtes sûr que vous n'avez pas besoin de cette information lors de l'examen de l'exception d'emballage alors c'est probablement correct.

+0

Je pense que cela dépend de la langue. Je sais avec certitude qu'en Java, vous pouvez "enchaîner" des exceptions de sorte que vous ne perdiez pas la pile-trace originale lorsque vous la relancez. –

0

Je ne suis pas sûr de PHP, mais par exemple. C# vous pouvez avoir plus d'un catch-Block, donc il n'y a pas besoin de combinaisons try/catch imbriquées.

Généralement je crois que la manipulation d'erreur avec try/catch/finally est toujours de bon sens, aussi pour montrer "seulement" une page d'erreur. C'est un moyen propre de gérer les erreurs et d'éviter les comportements étranges lors d'un crash.

+0

C# et PHP peuvent avoir plusieurs blocs catch pour un bloc try. –

2

L'idéal est que les exceptions soient interceptées au niveau qui peut les gérer. Pas avant (perte de temps), et pas après (vous perdez le contexte). Donc, si $ tpl et ERR_NOT_FOUND sont des informations qui ne sont "connues" que près du nouvel appel DM_Issue, par exemple parce qu'il y a d'autres endroits où vous créez un DM_Issue et où vous voulez ERR_SOMETHING_ELSE, ou parce que la valeur de $ tpl varie, alors vous attrapez la première exception au bon endroit.

Comment se rendre de cet endroit à mourir est une autre question. Une alternative serait de mourir là. Mais si vous faites cela, il n'y a aucune possibilité pour le code intervenant de faire quoi que ce soit (comme effacer quelque chose ou modifier la page d'erreur) après l'erreur mais avant de quitter. Il est également bon d'avoir un contrôle explicite. Donc je pense que tu es bon.

Je suppose que votre exemple n'est pas une application complète - si c'est le cas, il est probablement inutilement verbeux, et vous pourriez juste mourir dans la clause catch DM_Exception. Mais pour une vraie application, j'approuve le principe de ne pas simplement mourir au milieu de nulle part.

0

Je ne jette une exception question non trouvée - c'est un état valide d'une application, et vous ne pas besoin d'une trace de la pile juste pour afficher un 404.

Ce que vous devez attraper est inattendu les échecs, comme les erreurs sql - c'est à ce moment que la gestion des exceptions est pratique. Je voudrais changer votre code pour ressembler à ceci:

try { 
    $issue = DM_Issue::fetch($core->db->escape_string($_GET['issue'])); 
} 
catch (SQLException $e) { 
    log_error('SQL Error: DM_Issue::fetch()', $e->get_message()); 
} 
catch (Exception $e) { 
    log_error('Exception: DM_Issue::fetch()', $e->get_message()); 
} 

if(!$issue) { 
    display_error_page($tpl, ERR_NOT_FOUND); 
} 
else 
{ 
    // ... do stuff with $issue object. 
} 
0

Les exceptions devraient être utilisées que s'il y a un événement potentiellement rupture site - comme une requête de base de données n'exécute pas correctement ou quelque chose est mal configuré. Un bon exemple est qu'un cache ou un répertoire de journal n'est pas accessible en écriture par le processus Apache.

L'idée ici est que les exceptions sont pour vous, le développeur, d'arrêter le code qui peut casser le site entier afin que vous puissiez les réparer avant le déploiement. Ils sont également des vérifications de santé mentale pour s'assurer que si l'environnement change (c.-à-d. Quelqu'un modifie les permissions du dossier de cache ou change le schéma de base de données) le site est arrêté avant qu'il puisse endommager quoi que ce soit.

Donc, non; Les gestionnaires de captures imbriqués ne sont pas une bonne idée. Dans mes pages, mon fichier index.php enveloppe son code dans un bloc try ... cache - et si quelque chose de mal arrive, il vérifie s'il est en production ou non; et soit m'envoie un email et afficher une page d'erreur générique, ou montre l'erreur directement sur l'écran.

Rappelez-vous: PHP n'est pas C#. C# est (avec l'exception (hehe, sans jeu de mots: p) de ASP.net) pour les applications qui contiennent l'état - alors que PHP est un langage de script sans état.

Questions connexes