`trigger_error` vs` throw Exception` dans le contexte des méthodes magiques de PHP

13

J'ai un débat avec un collègue sur l'utilisation correcte (le cas échéant) de trigger_errordans le contexte des méthodes magiques . Premièrement, je pense que cela trigger_errordevrait être évité, sauf dans ce cas.

Disons que nous avons une classe avec une méthode foo()

class A {
    public function foo() {
        echo 'bar';
    }
}

Supposons maintenant que nous voulons fournir la même interface exacte, mais utilisons une méthode magique pour intercepter tous les appels de méthode

class B {
    public function __call($method, $args) {
        switch (strtolower($method)) {
        case 'foo':
            echo 'bar';
            break;
        }
    }
}

$a = new A;
$b = new B;

$a->foo(); //bar
$b->foo(); //bar

Les deux classes sont identiques dans la façon dont elles répondent, foo()mais diffèrent lors de l'appel d'une méthode non valide.

$a->doesntexist(); //Error
$b->doesntexist(); //Does nothing

Mon argument est que les méthodes magiques devraient appeler trigger_errorlorsqu'une méthode inconnue est interceptée

class B {
    public function __call($method, $args) {
        switch (strtolower($method)) {
        case 'foo':
            echo 'bar';
            break;
        default:
            $class = get_class($this);
            $trace = debug_backtrace();
            $file = $trace[0]['file'];
            $line = $trace[0]['line'];
            trigger_error("Call to undefined method $class::$method() in $file on line $line", E_USER_ERROR);
            break;
        }
    }
}

Pour que les deux classes se comportent (presque) de manière identique

$a->badMethod(); //Call to undefined method A::badMethod() in [..] on line 28
$b->badMethod(); //Call to undefined method B::badMethod() in [..] on line 32

Mon cas d'utilisation est une implémentation ActiveRecord. J'utilise __callpour attraper et gérer des méthodes qui font essentiellement la même chose mais qui ont des modificateurs comme Distinctou Ignore, par exemple

selectDistinct()
selectDistinctColumn($column, ..)
selectAll()
selectOne()
select()

ou

insert()
replace()
insertIgnore()
replaceIgnore()

Des méthodes telles que where(), from(), groupBy(), etc. sont codés en dur.

Mon argument est mis en évidence lorsque vous appelez accidentellement insret(). Si mon implémentation d'enregistrement actif codait en dur toutes les méthodes, ce serait une erreur.

Comme pour toute bonne abstraction, l'utilisateur doit ignorer les détails de mise en œuvre et se fier uniquement à l'interface. Pourquoi l'implémentation qui utilise des méthodes magiques devrait-elle se comporter différemment? Les deux devraient être une erreur.

chriso
la source

Réponses:

7

Prenez deux implémentations de la même interface ActiveRecord ( select(), where(), etc.)

class ActiveRecord1 {
    //Hardcodes all methods
}

class ActiveRecord2 {
    //Uses __call to handle some methods, hardcodes the rest
}

Si vous appelez une méthode non valide sur la première classe, par exemple ActiveRecord1::insret(), le comportement PHP par défaut est de déclencher une erreur . Un appel de fonction / méthode non valide n'est pas une condition qu'une application raisonnable voudrait intercepter et gérer. Bien sûr, vous pouvez l'attraper dans des langages comme Ruby ou Python où une erreur est une exception, mais d'autres (JavaScript / tout langage statique / plus?) Échoueront.

Revenons à PHP - si les deux classes implémentent la même interface, pourquoi ne devraient-elles pas présenter le même comportement?

Si __callou __callStaticdétectez une méthode invalide, elles devraient déclencher une erreur pour imiter le comportement par défaut de la langue

$class = get_class($this);
$trace = debug_backtrace();
$file = $trace[0]['file'];
$line = $trace[0]['line'];
trigger_error("Call to undefined method $class::$method() in $file on line $line", E_USER_ERROR);

Je ne dis pas si les erreurs doivent être utilisées par rapport aux exceptions (elles ne devraient pas 100%), mais je pense que les méthodes magiques de PHP sont une exception - jeu de mots :) - à cette règle dans le contexte du langage

chriso
la source
1
Désolé, mais je ne l'achète pas. Pourquoi devrions-nous être des moutons parce que les exceptions n'existaient pas en PHP il y a plus de dix ans lorsque les classes ont été implémentées pour la première fois en PHP 4.something?
Matthew Scharley
@Matthew pour la cohérence. Je ne discute pas des exceptions par rapport aux erreurs (il n'y a pas de débat) ou si PHP a une conception qui échoue (il le fait), je fais valoir que dans cette circonstance très unique, pour des raisons de cohérence , il est préférable d'imiter le comportement du langage
chriso
La cohérence n'est pas toujours une bonne chose. Une conception cohérente mais médiocre est toujours une mauvaise conception qui est pénible à utiliser en fin de journée.
Matthew Scharley
@Matthew true, mais IMO déclenchant une erreur sur un appel de méthode invalide n'est pas une mauvaise conception 1) beaucoup (la plupart?) De langues l'ont intégré, et 2) je ne peux pas penser à un seul cas où vous voudriez jamais pour attraper un appel de méthode invalide et le manipuler?
chriso
@chriso: L'univers est suffisamment grand pour qu'il y ait des cas d'utilisation que ni vous ni moi n'aurions jamais rêvé de se produire. Vous utilisez __call()pour faire du routage dynamique, est-il vraiment déraisonnable de s'attendre à ce que quelque part sur la piste, quelqu'un veuille gérer le cas où cela échoue? En tout cas, cela tourne en rond, donc ce sera mon dernier commentaire. Faites ce que vous voulez, en fin de compte, cela revient à un appel au jugement: un meilleur soutien par rapport à la cohérence. Les deux méthodes auront le même effet sur l'application en cas d'absence de manipulation particulière.
Matthew Scharley
3

Je vais jeter mon opinion d'opinion, mais si vous utilisez trigger_errorn'importe où, vous faites quelque chose de mal. Les exceptions sont la voie à suivre.

Avantages des exceptions:

  • Ils peuvent être capturés. C'est un énorme avantage et devrait être le seul dont vous avez besoin. Les gens peuvent essayer quelque chose de différent s'ils s'attendent à ce que quelque chose se passe mal. L'erreur ne vous donne pas cette opportunité. Même la configuration d'un gestionnaire d'erreurs personnalisé ne tient pas la peine de simplement attraper une exception. En ce qui concerne vos commentaires dans votre question, ce qu'est une demande «raisonnable» dépend entièrement du contexte de la demande. Les gens ne peuvent pas attraper d'exceptions s'ils pensent que cela n'arrivera jamais dans leur cas. Ne pas donner aux gens le choix est une mauvaise chose ™.
  • Tracez des traces. En cas de problème, vous savez et dans quel contexte le problème s'est produit. Avez-vous déjà essayé de localiser une erreur provenant de certaines des méthodes principales? Si vous appelez une fonction avec trop peu de paramètres, vous obtenez une erreur inutile qui met en évidence le début de la méthode que vous appelez et ignore totalement d'où elle appelle.
  • Clarté. Combinez les deux ci-dessus et vous obtenez un code plus clair. Si vous essayez d'utiliser un gestionnaire d'erreurs personnalisé pour gérer les erreurs (par exemple, pour générer des traces de pile pour les erreurs), toute votre gestion des erreurs se fait dans une seule fonction, pas là où les erreurs sont réellement générées.

Répondre à vos préoccupations, appeler une méthode qui n'existe pas peut être une possibilité valable . Cela dépend entièrement du contexte du code que vous écrivez, mais il peut arriver que cela se produise dans certains cas. Pour répondre à votre cas d'utilisation exact, certains serveurs de base de données peuvent autoriser certaines fonctionnalités que d'autres ne permettent pas. L'utilisation de try/ catchet des exceptions dans __call()vs. une fonction pour vérifier les capacités est un tout autre argument.

Le seul cas d'utilisation auquel je peux penser trigger_errorest pour E_USER_WARNINGou inférieur. Déclencher un E_USER_ERRORbien est toujours une erreur à mon avis.

Matthew Scharley
la source
Merci pour la réponse :) - 1) Je suis d'accord que les exceptions devraient presque toujours être utilisées pour les erreurs - tous vos arguments sont des points valides. Cependant .. je pense que dans ce contexte votre argument échoue ..
chriso
2
" appeler une méthode qui n'existe pas peut être une possibilité valable " - je suis complètement en désaccord. Dans tous les langages (que j'ai jamais utilisés), appeler une fonction / méthode qui n'existe pas entraînera une erreur. Ce n'est pas une condition qui devrait être détectée et manipulée. Les langages statiques ne vous permettront pas de compiler avec un appel de méthode non valide et les langages dynamiques échoueront une fois qu'ils auront atteint l'appel.
chriso
Si vous lisez mon deuxième montage, vous verrez mon cas d'utilisation. Supposons que vous ayez deux implémentations de la même classe, l'une utilisant __call et l'autre avec des méthodes codées en dur. Ignorant les détails de l'implémentation, pourquoi les deux classes devraient-elles se comporter différemment lorsqu'elles implémentent la même interface? PHP déclenchera une erreur si vous appelez une méthode non valide avec la classe qui a des méthodes codées en dur. Utiliser trigger_errordans le contexte de __call ou __callStatic imite le comportement par défaut de la langue
chriso
@chriso: les langages dynamiques qui ne sont pas PHP échoueront avec une exception qui peut être interceptée . Ruby, par exemple, en lance un NoMethodErrorque vous pouvez attraper si vous le souhaitez. Les erreurs sont une erreur géante en PHP à mon avis personnel. Ce n'est pas parce que le noyau utilise une méthode cassée pour signaler des erreurs que votre propre code devrait l'être.
Matthew Scharley
@chriso J'aime croire que la seule raison pour laquelle le noyau utilise toujours des erreurs est la compatibilité descendante. Espérons que PHP6 sera un autre bond en avant comme PHP5 et supprimera complètement les erreurs. À la fin de la journée, les erreurs et les exceptions génèrent le même résultat: l'arrêt immédiat de l'exécution du code. À une exception près, vous pouvez diagnostiquer pourquoi et où c'est beaucoup plus facile.
Matthew Scharley
3

Les erreurs PHP standard doivent être considérées comme obsolètes. PHP fournit une classe intégrée ErrorException pour convertir les erreurs, avertissements et notifications en exceptions avec une trace de pile appropriée. Vous l'utilisez comme ceci:

function errorToExceptionHandler($errNo, $errStr, $errFile, $errLine, $errContext)
{
if (error_reporting() == 0) return;
throw new ErrorException($errStr, 0, $errNo, $errFile, $errLine);
}
set_error_handler('errorToExceptionHandler');

En utilisant cela, cette question devient théorique. Les erreurs intégrées déclenchent désormais des exceptions et votre propre code devrait également l'être.

Wayne
la source
1
Si vous l'utilisez, vous devez vérifier le type de l'erreur, de peur de transformer les E_NOTICEs en exceptions. Ce serait mauvais.
Matthew Scharley
1
Non, transformer E_NOTICES en exceptions est bien! Tous les avis doivent être considérés comme des erreurs; c'est une bonne pratique que vous les convertissiez en exceptions ou non.
Wayne
2
Normalement, je suis d'accord avec vous, mais à la seconde où vous commencez à utiliser un code tiers, cela a tendance à tomber rapidement.
Matthew Scharley
Presque toutes les bibliothèques tierces sont E_STRICT | E_ALL sûr. Si j'utilise du code qui ne l'est pas, je vais le réparer. J'ai travaillé de cette façon pendant des années sans problème.
Wayne
vous n'avez évidemment jamais utilisé Dual auparavant. Le noyau est vraiment bon comme ça mais beaucoup, beaucoup de modules tiers ne le sont pas
Matthew Scharley
0

OMI, c'est un cas d'utilisation parfaitement valable pour trigger_error:

function handleError($errno, $errstring, $errfile, $errline, $errcontext) {
    if (error_reporting() & $errno) {
        // only process when included in error_reporting
        return handleException(new \Exception($errstring, $errno));
    }
    return true;
}

function handleException($exception){
    // Here, you do whatever you want with the generated
    // exceptions. You can store them in a file or database,
    // output them in a debug section of your page or do
    // pretty much anything else with it, as if it's a
    // normal variable

    switch ($code) {
        case E_ERROR:
        case E_CORE_ERROR:
        case E_USER_ERROR:
            // Make sure script exits here
            exit(1);
        default:
            // Let script continue
            return true;
    }
}

// Set error handler to your custom handler
set_error_handler('handleError');
// Set exception handler to your custom handler
set_exception_handler('handleException');


// ---------------------------------- //

// Generate warning
trigger_error('This went wrong, but we can continue', E_USER_WARNING);

// Generate fatal error :
trigger_error('This went horrible wrong', E_USER_ERROR);

En utilisant cette stratégie, vous obtenez le $errcontext paramètre si vous le faites $exception->getTrace()dans la fonction handleException. Ceci est très utile à certaines fins de débogage.

Malheureusement, cela ne fonctionne que si vous utilisez trigger_error directement à partir de votre contexte, ce qui signifie que vous ne pouvez pas utiliser une fonction / méthode wrapper pour alias la trigger_errorfonction (vous ne pouvez donc pas faire quelque chose comme function debug($code, $message) { return trigger_error($message, $code); }si vous voulez les données de contexte dans votre trace).

Je cherchais une meilleure alternative, mais jusqu'à présent je n'en ai pas trouvé.

John Slegers
la source