Avoir un drapeau pour indiquer s'il faut jeter des erreurs

64

J'ai récemment commencé à travailler dans un endroit avec des développeurs beaucoup plus âgés (environ 50 ans et plus). Ils ont travaillé sur des applications critiques de l’aviation où le système ne pouvait pas tomber en panne. En conséquence, le programmeur le plus âgé a tendance à coder de cette façon.

Il a tendance à mettre un booléen dans les objets pour indiquer si une exception doit être levée ou non.

Exemple

public class AreaCalculator
{
    AreaCalculator(bool shouldThrowExceptions) { ... }
    CalculateArea(int x, int y)
    {
        if(x < 0 || y < 0)
        {
            if(shouldThrowExceptions) 
                throwException;
            else
                return 0;
        }
    }
}

(Dans notre projet, la méthode peut échouer car nous essayons d'utiliser un périphérique réseau qui ne peut pas être présent à ce moment-là. L'exemple de zone n'est qu'un exemple de l'indicateur d'exception)

Pour moi, cela ressemble à une odeur de code. L'écriture de tests unitaires devient légèrement plus complexe car vous devez tester l'indicateur d'exception à chaque fois. De plus, si quelque chose ne va pas, ne voudriez-vous pas savoir tout de suite? L'appelant ne devrait-il pas avoir la responsabilité de déterminer comment continuer?

Sa logique / raisonnement est que notre programme doit faire une chose, montrer des données à l'utilisateur. Toute autre exception qui ne nous empêche pas de le faire doit être ignorée. Je conviens qu’ils ne devraient pas être ignorés, mais devraient faire l’objet d’une bulle, être gérés par la personne appropriée, et ne pas avoir à traiter avec des drapeaux pour cela.

Est-ce une bonne façon de gérer les exceptions?

Edit : Juste pour donner plus de contexte à la décision de conception, je suppose que c’est parce que si ce composant échoue, le programme peut toujours fonctionner et s’acquitter de sa tâche principale. Ainsi, nous ne voudrions pas lancer une exception (et ne pas la gérer?) Et la supprimer du programme lorsque, pour l'utilisateur, cela fonctionne bien

Edit 2 : Pour donner encore plus de contexte, dans notre cas, la méthode est appelée pour réinitialiser une carte réseau. Le problème se pose lorsque la carte réseau est déconnectée et reconnectée, une adresse IP différente lui est attribuée. Réinitialisation générera une exception car nous essayons de réinitialiser le matériel avec l'ancienne adresse IP.

Nicolas
la source
22
c # a une convention pour ce modèle Try-Parse. Plus d'infos: docs.microsoft.com/en-us/dotnet/standard/design-guidelines/… Le drapeau ne correspond pas à ce modèle.
Peter
18
Il s’agit essentiellement d’un paramètre de contrôle qui modifie l’exécution des méthodes internes. C'est mauvais, peu importe le scénario. martinfowler.com/bliki/FlagArgument.html , softwareengineering.stackexchange.com/questions/147977/… , medium.com/@amlcurran/…
bic Le
1
En plus du commentaire Try-Parse de Peter, voici un bel article sur les exceptions Vexing
Linaith
2
"Ainsi, nous ne voudrions pas lancer une exception (et non la gérer?) Et le supprimer du programme lorsque cela fonctionnera pour l'utilisateur" - vous savez que vous pouvez intercepter des exceptions, n'est-ce pas?
user253751
1
Je suis à peu près sûr que cela a déjà été abordé ailleurs, mais étant donné le simple exemple Area, je serais plus enclin à me demander d'où proviendraient ces nombres négatifs et si vous seriez capable de gérer cette condition d'erreur ailleurs (par exemple, quel que soit ce qui lisait le fichier contenant longueur et largeur, par exemple); Toutefois, "nous essayons d'utiliser un périphérique réseau qui ne peut pas être présent à ce moment-là". Ce point pourrait mériter une réponse totalement différente. S'agit-il d'une API tierce ou d'un standard tel que TCP / UDP?
Jrh

Réponses:

73

Le problème avec cette approche est que, même si des exceptions ne sont jamais levées (et donc, l’application ne se bloque jamais en raison d’exceptions non capturées), les résultats renvoyés ne sont pas nécessairement corrects et il se peut que l’utilisateur ne sache jamais qu’il ya un problème avec les données (ou quel est ce problème et comment le corriger).

Pour que les résultats soient corrects et significatifs, la méthode appelante doit rechercher des nombres spéciaux dans le résultat, c'est-à-dire des valeurs de retour spécifiques utilisées pour signaler les problèmes survenus lors de l'exécution de la méthode. Les nombres négatifs (ou zéro) renvoyés pour des quantités définies positives (comme la surface) en sont un excellent exemple dans un code plus ancien. Si la méthode appelante ne sait pas (ou n'oublie pas!) De vérifier ces numéros spéciaux, le traitement peut continuer sans que personne ne se rende compte de l'erreur. Les données sont ensuite affichées à l'utilisateur, montrant une zone de 0, que l'utilisateur sait être incorrecte, mais qui n'ont aucune indication sur ce qui a mal tourné, où et pourquoi. Ils se demandent alors si l'une des autres valeurs est fausse ...

Si l’exception était levée, le traitement s’arrêterait, l’erreur serait (idéalement) consignée et l’utilisateur pourrait en être averti d’une manière ou d’une autre. L'utilisateur peut alors réparer ce qui ne va pas et réessayer. Une gestion correcte des exceptions (et des tests!) Garantira que les applications critiques ne se bloquent pas ou ne finissent pas dans un état non valide.

mmathis
la source
1
@Quirk C'est impressionnant de constater que Chen a réussi à violer le principe de responsabilité unique en seulement 3 ou 4 lignes. C'est le vrai problème. De plus, le problème dont il parle (le programmeur ne réfléchissant pas aux conséquences des erreurs sur chaque ligne) est toujours une possibilité avec des exceptions non vérifiées et seulement parfois une possibilité avec des exceptions vérifiées. Je pense avoir déjà vu tous les arguments avancés contre les exceptions vérifiées, et aucun d'entre eux n'est valide.
No U
@TKK personnellement, il y a des cas dans lesquels j'aurais vraiment aimé des exceptions vérifiées dans .NET. Il serait bien qu’il existe des outils d’analyse statique haut de gamme qui permettent de s’assurer de la précision des documents de l’API en tant qu’exceptions émises, bien que cela soit probablement pratiquement impossible, en particulier lors de l’accès à des ressources natives.
HJR
1
@jrh Oui, ce serait bien si quelque chose supprimait la sécurité d'exception dans .NET, de la même manière que TypeScript respecte la sécurité dans JS.
No U
47

Est-ce une bonne façon de gérer les exceptions?

Non, je pense que c'est une très mauvaise pratique. Lancer une exception ou renvoyer une valeur est un changement fondamental de l'API, qui modifie la signature de la méthode et lui donne un comportement tout à fait différent du point de vue de l'interface.

En général, lorsque nous concevons des classes et leurs API, nous devrions considérer que

  1. il peut y avoir plusieurs instances de la classe avec différentes configurations flottant dans le même programme au même moment, et,

  2. En raison de l'injection de dépendance et de nombreuses autres pratiques de programmation, un client consommateur peut créer les objets et les confier à un autre utilisateur, ce qui entraîne souvent une séparation entre les créateurs et les utilisateurs d'objets.

Voyons maintenant ce que l’appelant doit faire pour utiliser une instance qui lui a été attribuée, par exemple, pour appeler la méthode de calcul: l’appelant devra à la fois vérifier la surface zéro et capturer les exceptions - ouch! Les considérations relatives aux tests ne concernent pas seulement la classe elle-même, mais également la gestion des erreurs des appelants ...

Nous devrions toujours rendre les choses aussi simples que possible pour le client consommateur; cette configuration booléenne dans le constructeur qui modifie l'API d'une méthode d'instance est le contraire qui fait que le programmeur client consommateur (peut-être vous ou votre collègue) tombe dans le gouffre du succès.

Pour offrir les deux API, il est bien mieux et plus normal de fournir deux classes différentes: une classe générant toujours une erreur et une autre renvoyant toujours 0 en cas d'erreur, ou bien deux méthodes différentes avec une seule classe. De cette manière, le client consommateur peut facilement savoir exactement comment vérifier et gérer les erreurs.

En utilisant deux classes différentes ou deux méthodes différentes, vous pouvez utiliser beaucoup plus facilement les utilisateurs de méthodes de recherche IDE et les fonctions de refactorisation, etc., car les deux cas d'utilisation ne sont plus combinés. La lecture, l'écriture, la maintenance, les revues et les tests de codes sont également plus simples.


Sur une autre note, j'estime personnellement que nous ne devrions pas prendre les paramètres de configuration booléens, où les appelants passent tous simplement une constante . Un tel paramétrage de configuration associe deux cas d'utilisation distincts sans aucun avantage réel.

Examinez votre base de code et voyez si une variable (ou une expression non constante) est utilisée pour le paramètre de configuration boolean dans le constructeur! J'en doute.


Et une autre considération est de demander pourquoi le calcul de la zone peut échouer. Le mieux serait peut-être de jeter le constructeur, si le calcul ne peut pas être fait. Toutefois, si vous ne savez pas si le calcul peut être effectué jusqu'à ce que l'objet soit initialisé davantage, envisagez peut-être d'utiliser différentes classes pour différencier ces états (non prêt à calculer une surface ou prêt à calculer).

J'ai lu que votre situation d'échec est orientée vers la communication à distance, donc peut ne pas s'appliquer; Seulement un peu de matière à réflexion.


L'appelant ne devrait-il pas avoir la responsabilité de déterminer comment continuer?

Oui je suis d'accord. Il semble prématuré pour l'appelé de décider que la zone 0 est la bonne réponse dans les conditions d'erreur (d'autant plus que 0 est une zone valide et qu'il est donc impossible de faire la différence entre une erreur et le 0 réel, même si elle ne s'applique pas à votre application).

Erik Eidt
la source
Vous n'avez pas du tout besoin de vérifier l'exception car vous devez vérifier les arguments avant d' appeler la méthode. La vérification du résultat par rapport à zéro ne fait pas la distinction entre les arguments juridiques 0, 0 et les arguments négatifs illégaux. L'API est vraiment horrible à mon humble avis.
BlackJack
Les poussées MS de l’Annexe K pour les iostreams C99 et C ++ sont des exemples d’API dans lesquels un crochet ou un indicateur modifie radicalement la réaction aux pannes.
Déduplicateur
37

Ils ont travaillé sur des applications critiques de l’aviation où le système ne pouvait pas tomber en panne. Par conséquent ...

C'est une introduction intéressante, ce qui me donne l'impression que la motivation derrière cette conception est d'éviter de jeter des exceptions dans certains contextes "parce que le système pourrait tomber en panne" . Mais si le système "peut tomber en panne à cause d'une exception", cela indique clairement que

  • les exceptions ne sont pas traitées correctement , du moins ni de manière rigide.

Donc, si le programme qui l'utilise AreaCalculatorest bogué, votre collègue préfère ne pas le faire "planter tôt", mais renvoyer une valeur erronée (en espérant que personne ne le remarque, ou en espérant que personne ne fasse quelque chose d'important avec cela). En réalité, cela masque une erreur et, d’après mon expérience, cela conduira tôt ou tard à des bogues de suivi pour lesquels il devient difficile de trouver la cause fondamentale.

À mon humble avis, écrire un programme qui ne plante pas dans toutes les circonstances mais qui présente des données erronées ou des résultats de calcul erronés ne vaut généralement pas mieux que de laisser le programme planter. La seule approche appropriée consiste à donner à l'appelant une chance de remarquer l'erreur, de la gérer, de lui permettre de décider si l'utilisateur doit être informé du mauvais comportement et s'il est prudent de poursuivre tout traitement ou s'il est plus sûr. pour arrêter complètement le programme. Ainsi, je recommanderais l’un des suivants:

  • il est difficile d’ignorer le fait qu’une fonction peut générer une exception. La documentation et les normes de codage sont vos amis, et les révisions de code régulières devraient permettre une utilisation correcte des composants et une gestion correcte des exceptions.

  • formez l'équipe à attendre et à gérer les exceptions lorsqu'elles utilisent des composants de "boîte noire" et à prendre en compte le comportement global du programme.

  • si pour certaines raisons vous pensez que vous ne pouvez pas obtenir le code appelant (ou les développeurs qui l'écrivent) pour utiliser correctement la gestion des exceptions, vous pouvez, en dernier recours, concevoir une API avec des variables de sortie d'erreur explicites et aucune exception, comme

    CalculateArea(int x, int y, out ErrorCode err)

    il est donc très difficile pour l’appelant d’ignorer que la fonction risque d’échouer. Mais c'est IMHO extrêmement moche en C #; c'est une vieille technique de programmation défensive de C qui ne fait pas exception à la règle et il ne devrait normalement pas être nécessaire de travailler de cette façon.

Doc Brown
la source
3
"écrire un programme qui ne plante pas, quelles que soient les circonstances, mais qui présente des données erronées ou des résultats de calcul erronés ne vaut généralement pas mieux que de laisser le programme planter." toujours avec les instruments affichant des valeurs erronées par rapport à l’arrêt de l’ordinateur de l’avion. Pour toutes les applications moins critiques, il est définitivement préférable de ne pas masquer les erreurs.
Trilarion le
18
@Trilarion: si un programme pour ordinateur de vol ne contient pas une gestion correcte des exceptions, la "correction de ce problème" en faisant en sorte que les composants ne génèrent pas d'exceptions est une approche très peu judicieuse. Si le programme se bloque, il devrait y avoir un système de sauvegarde redondant qui peut prendre le relais. Si le programme ne plante pas et affiche une mauvaise hauteur, par exemple, les pilotes peuvent penser que "tout va bien" pendant que l'avion se précipite dans la montagne suivante.
Doc Brown le
7
@Trilarion: si l'ordinateur de vol affiche une hauteur incorrecte et que l'avion va s'écraser, cela ne vous aidera pas non plus (surtout lorsque le système de secours est présent et qu'il n'est pas informé qu'il doit prendre le relais). Les systèmes de sauvegarde pour les ordinateurs d’avion n’est pas une idée nouvelle, google pour les "systèmes de sauvegarde d’ordinateur d’avion", je suis assez sûr que les ingénieurs du monde entier ont toujours construit des systèmes redondants dans tout système réellement critique Assurance).
Doc Brown le
4
Cette. Si vous ne pouvez pas vous permettre que le programme se bloque, vous ne pouvez pas non plus vous permettre de le laisser répondre en silence. La bonne réponse est d'avoir un traitement approprié des exceptions dans tous les cas. Pour un site Web, cela signifie un gestionnaire global qui convertit des erreurs inattendues en 500. Vous pouvez également avoir des gestionnaires supplémentaires pour des situations plus spécifiques, comme avoir un try/ catchintérieur d' une boucle si vous avez besoin de traitement pour continuer si un élément échoue.
jpmc26
2
Obtenir le mauvais résultat est toujours le pire type d’échec; cela me rappelle la règle sur l'optimisation: "corrigez-vous avant d'optimiser, car obtenir la mauvaise réponse plus rapidement ne profite toujours à personne."
Toby Speight le
13

L'écriture de tests unitaires devient légèrement plus complexe car vous devez tester l'indicateur d'exception à chaque fois.

Toute fonction avec n paramètres sera plus difficile à tester qu'une fonction avec n-1 paramètres. Étendez cela à l'absurde et l'argument devient que les fonctions ne devraient pas avoir de paramètres du tout, car cela les rend plus faciles à tester.

Bien que ce soit une bonne idée d’écrire du code facile à tester, c’est une idée terrible de placer la simplicité du test avant l’écriture de code utile pour ceux qui doivent l’appeler. Si l'exemple dans la question comporte un commutateur qui détermine si une exception est levée ou non, il est possible que le nombre d'appelants souhaitant ce comportement ait mérité de l'ajouter à la fonction. Là où la limite entre complexe et trop complexe se situe est un jugement; quiconque essaie de vous dire qu'il y a une ligne de démarcation qui s'applique dans toutes les situations devrait être regardé avec suspicion.

De plus, si quelque chose ne va pas, ne voudriez-vous pas savoir tout de suite?

Cela dépend de votre définition du mal. L'exemple de la question définit "incorrect" comme "étant donné une dimension inférieure à zéro et shouldThrowExceptionsvraie". Etre donné une dimension si moins que zéro n'est pas faux quand shouldThrowExceptionsest faux parce que le commutateur induit un comportement différent. C'est tout simplement pas une situation exceptionnelle.

Le vrai problème ici est que le commutateur a été mal nommé car il ne décrit pas ce que la fonction fait. Si on lui avait donné un meilleur nom treatInvalidDimensionsAsZero, aurais-tu posé cette question?

L'appelant ne devrait-il pas avoir la responsabilité de déterminer comment continuer?

L'appelant ne détermine comment continuer. Dans ce cas, il le fait à l'avance en définissant ou en effaçant shouldThrowExceptionset la fonction se comporte en fonction de son état.

L'exemple est pathologiquement simple car il effectue un seul calcul et retourne. Si vous le rendez un peu plus complexe, tel que le calcul de la somme des racines carrées d'une liste de numéros, le lancement d'exceptions peut donner aux appelants des problèmes qu'ils ne peuvent pas résoudre. Si je transmets une liste de [5, 6, -1, 8, 12]et que la fonction jette une exception sur le -1, je n'ai aucun moyen de dire à la fonction de continuer car elle aura déjà été annulée et jeté la somme. Si la liste est un ensemble de données énorme, générer une copie sans aucun nombre négatif avant d'appeler la fonction risque de ne pas être pratique. Je suis donc obligé de dire à l'avance comment les nombres non valides doivent être traités, soit sous la forme d'un "ignorez-les simplement." "changer ou peut-être fournir un lambda qui est appelé à prendre cette décision.

Sa logique / raisonnement est que notre programme doit faire une chose, montrer des données à l'utilisateur. Toute autre exception qui ne nous empêche pas de le faire doit être ignorée. Je conviens qu’ils ne devraient pas être ignorés, mais devraient faire l’objet d’une bulle, être gérés par la personne appropriée, et ne pas avoir à traiter avec des drapeaux pour cela.

Encore une fois, il n’existe pas de solution unique. Dans cet exemple, la fonction a vraisemblablement été écrite dans une spécification indiquant comment traiter les dimensions négatives. La dernière chose que vous souhaitiez faire est de réduire le rapport signal sur bruit de vos journaux en les remplissant de messages indiquant "normalement, une exception serait levée ici, mais l'appelant a dit de ne pas déranger."

Et en tant que programmeur beaucoup plus âgé, je vous demanderais de bien vouloir quitter votre pelouse. ;-)

Blrfl
la source
Je conviens que la dénomination et l'intention sont extrêmement importantes et que, dans ce cas, un nom de paramètre correct peut réellement faire basculer les tables. +1, mais l' our program needs to do 1 thing, show data to user. Any other exception that doesn't stop us from doing so should be ignoredétat d'esprit peut amener l'utilisateur à prendre une décision en se basant sur des données erronées (car le programme doit en réalité le faire 1 chose - aider l'utilisateur à prendre des décisions éclairées) 2. des cas similaires comme ceux-ci bool ExecuteJob(bool throwOnError = false)sont généralement extrêmement sujettes aux erreurs et conduisent à un code qu'il est difficile de raisonner en le lisant simplement.
Eugene Podskal
@EugenePodskal Je pense que l'hypothèse est que "montrer les données" signifie "montrer les données correctes". Le questionneur ne dit pas que le produit fini ne fonctionne pas, mais simplement qu'il pourrait être écrit «faux». Il faudrait que je voie des données concrètes sur le deuxième point. Dans mon projet actuel, j'ai une poignée de fonctions très utilisées qui ont un commutateur de projection / non-projection et ne sont pas plus difficiles à raisonner que toute autre fonction, mais ce n'est qu'un point de données.
Blrfl
Bonne réponse, je pense que cette logique s’applique à un éventail de circonstances beaucoup plus vaste que les seuls PO. BTW, le nouveau cpp a une version jet / non-lancer pour exactement ces raisons. Je sais qu'il y a quelques petites différences mais ...
drjpizzle
8

Les codes critiques et «normaux» en matière de sécurité peuvent donner lieu à des idées très différentes sur la «bonne pratique». Il y a beaucoup de chevauchement - certaines choses sont risquées et devraient être évitées dans les deux cas - mais il y a toujours des différences significatives. Si vous ajoutez une exigence pour garantir la réactivité, ces écarts deviennent assez importants.

Ceux-ci se rapportent souvent à des choses que vous attendez:

  • Pour git, la mauvaise réponse pourrait être très mauvaise par rapport à: prendre trop de temps / abandonner / se suspendre ou même se planter (ce sont en réalité des problèmes qui ne concernent pas, par exemple, la modification accidentelle du code d’enregistrement).

    Toutefois, un tableau de bord dont le calcul de la force g doit caler et empêcher un calcul de la vitesse de l’air peut être inacceptable.

Certains sont moins évidents:

  • Si vous avez beaucoup testé , les résultats de premier ordre (comme les bonnes réponses) ne vous inquiétez pas autant. Vous savez que vos tests auront couvert cela. Cependant, s'il y avait un état caché ou un flux de contrôle, vous ne savez pas que cela ne sera pas la cause de quelque chose de beaucoup plus subtil. C'est difficile à exclure avec des tests.

  • Être manifestement en sécurité est relativement important. Peu de clients se demanderont si la source qu'ils achètent est sûre ou non. Si vous êtes dans le marché de l'aviation, d'autre part ...

Comment cela s'applique-t-il à votre exemple:

Je ne sais pas. Il existe un certain nombre de processus de réflexion pouvant comporter des règles principales telles que "aucun code de production" adoptées dans un code critique pour la sécurité qui seraient assez ridicules dans des situations plus habituelles.

Certaines concernent l’intégration, certaines la sécurité et peut-être d’autres… Certaines sont bonnes (performances serrées / limites de mémoire nécessaires), d’autres sont mauvaises (nous ne gérons pas les exceptions correctement, il est donc préférable de ne pas la risquer). La plupart du temps, même savoir pourquoi ils l'ont fait ne répondra pas vraiment à la question. Par exemple, s’il s’agit de faciliter la vérification du code et d’améliorer le code, est-ce une bonne pratique? Vous ne pouvez vraiment pas dire. Ce sont des animaux différents, et doivent être traités différemment.

Cela dit, cela me semble un peu suspect, MAIS :

Les logiciels critiques pour la sécurité et les décisions en matière de conception de logiciels ne devraient probablement pas être pris par des inconnus lors de l'échange de pile-à-vis des logiciels. Il y a peut-être une bonne raison de faire cela même si cela fait partie d'un mauvais système. Ne lisez pas trop dans autre chose que comme "matière à réflexion".

droguer
la source
7

Parfois, lancer une exception n'est pas la meilleure méthode. Notamment en raison du dépilage de pile, mais parfois parce que capturer une exception est problématique, en particulier le long des coutures de langage ou d'interface.

Un bon moyen de gérer cela consiste à renvoyer un type de données enrichi. Ce type de données a assez d'état pour décrire tous les chemins heureux et tous les chemins malheureux. Le fait est que si vous interagissez avec cette fonction (membre / global / sinon), vous serez obligé de gérer le résultat.

Cela étant dit, ce type de données enrichi ne devrait pas forcer l'action. Imaginez dans votre région par exemple quelque chose comme var area_calc = new AreaCalculator(); var volume = area_calc.CalculateArea(x, y) * z;. Cela semble utile volumedevrait contenir la surface multipliée par la profondeur - ce qui pourrait être un cube, un cylindre, etc.

Mais que se passe-t-il si le service area_calc est en panne? Puis area_calc .CalculateArea(x, y)renvoyé un type de données riche contenant une erreur. Est-il légal de multiplier cela par z? C'est une bonne question. Vous pouvez forcer les utilisateurs à gérer la vérification immédiatement. Cela brise toutefois la logique avec la gestion des erreurs.

var area_calc = new AreaCalculator();
var area_result = area_calc.CalculateArea(x, y);
if (area_result.bad())
{
    //handle unhappy path
}
var volume = area_result.value() * z;

contre

var area_calc = new AreaCalculator();
var volume = area_calc.CalculateArea(x, y) * z;
if (volume.bad())
{
    //handle unhappy path
}

La logique essentielle est répartie sur deux lignes et divisée par le traitement des erreurs dans le premier cas, tandis que le second cas contient toute la logique pertinente sur une ligne, suivie du traitement des erreurs.

Dans ce second cas, il volumes'agit d'un type de données riche. Ce n'est pas juste un chiffre. Cela rend le stockage plus volumineux et volumenécessitera toujours une enquête pour détecter une condition d'erreur. De plus, il est possible volumeque d'autres calculs soient fournis avant que l'utilisateur ne décide de gérer l'erreur, ce qui lui permet de se manifester à plusieurs endroits différents. Cela peut être bon ou mauvais en fonction des spécificités de la situation.

Alternativement, il volumepourrait ne s'agir que d'un simple type de données, d'un simple nombre, mais qu'adviendra-t-il de la condition d'erreur? Il se peut que la valeur convertisse implicitement si elle est dans un état heureux. En cas de malheur, il peut renvoyer une valeur par défaut / une erreur (pour la zone 0 ou -1, cela peut sembler raisonnable). Sinon, une exception peut être levée de ce côté de la frontière interface / langue.

... foo() {
   var area_calc = new AreaCalculator();
   return area_calc.CalculateArea(x, y) * z;
}
var volume = foo();
if (volume <= 0)
{
    //handle error
}

contre.

... foo() {
   var area_calc = new AreaCalculator();
   return area_calc.CalculateArea(x, y) * z;
}

try { var volume = foo(); }
catch(...)
{
    //handle error
}

En transmettant une valeur mauvaise ou éventuellement mauvaise, il incombe beaucoup à l'utilisateur de valider les données. C'est une source de bogues, car pour le compilateur, la valeur de retour est un entier légitime. Si quelque chose n'a pas été vérifié, vous le découvrirez en cas de problème. Le second cas mêle le meilleur des deux mondes en permettant aux exceptions de gérer les chemins malheureux, tandis que les chemins heureux suivent un traitement normal. Malheureusement, cela oblige l'utilisateur à gérer les exceptions avec sagesse, ce qui est difficile.

Pour être clair, un chemin malheureux est un cas inconnu de la logique métier (le domaine de l’exception), ne pas valider est un chemin heureux, car vous savez comment le gérer par les règles commerciales (le domaine des règles).

La solution ultime serait celle qui autorise tous les scénarios (dans des limites raisonnables).

  • L'utilisateur doit pouvoir rechercher une mauvaise condition et la traiter immédiatement.
  • L'utilisateur doit pouvoir utiliser le type enrichi comme si le chemin d'accès satisfaisant avait été suivi et propager les détails de l'erreur.
  • L'utilisateur doit pouvoir extraire la valeur du chemin heureux par transtypage (implicite / explicite dans la mesure du raisonnable), générant une exception pour les chemins malheureux.
  • L'utilisateur doit pouvoir extraire la valeur du chemin heureux ou utiliser une valeur par défaut (fournie ou non).

Quelque chose comme:

Rich::value_type value_or_default(Rich&, Rich::value_type default_value = ...);
bool bad(Rich&);
...unhappy path report... bad_state(Rich&);
Rich& assert_not_bad(Rich&);
class Rich
{
public:
   typedef ... value_type;

   operator value_type() { assert_not_bad(*this); return ...value...; }
   operator X(...) { if (bad(*this)) return ...propagate badness to new value...; /*operate and generate new value*/; }
}

//check
if (bad(x))
{
    var report = bad_state(x);
    //handle error
}

//rethrow
assert_not_bad(x);
var result = (assert_not_bad(x) + 23) / 45;

//propogate
var y = x * 23;

//implicit throw
Rich::value_type val = x;
var val = ((Rich::value_type)x) + 34;
var val2 = static_cast<Rich::value_type>(x) % 3;

//default value
var defaulted = value_or_default(x);
var defaulted_to = value_or_default(x, 55);
Kain0_0
la source
@TobySpeight Assez, ces choses sont sensibles au contexte et ont une portée étendue.
Kain0_0
Je pense que le problème ici est les blocs 'assert_not_bad'. Je pense que ceux-ci finiront au même endroit que le code original a tenté de résoudre. Lors des tests, il convient de les noter, mais s’ils sont réellement affirmés, ils doivent être supprimés avant la production sur un véritable avion. sinon quelques bons points.
drjpizzle
@drjpizzle Je dirais que s'il était assez important d'ajouter un protecteur pour les tests, il l'est suffisamment pour le laisser en place lors de l'exécution en production. La présence de la garde elle-même implique le doute. Si vous doutez suffisamment du code pour le protéger lors des tests, vous le doutez pour une raison technique. c.-à-d. que la condition pourrait / se produit de manière réaliste L'exécution de tests ne prouve pas que la condition ne sera jamais atteinte en production. Cela signifie qu’une condition connue, susceptible de se produire, doit être traitée quelque part. Je pense que c'est le problème qui se pose.
Kain0_0
3

Je vais répondre du point de vue de C ++. Je suis à peu près sûr que tous les concepts de base sont transférables en C #.

On dirait que votre style préféré est "jetez toujours des exceptions":

int CalculateArea(int x, int y) {
    if (x < 0 || y < 0) {
        throw Exception("negative side lengths");
    }
    return x * y;
}

Cela peut poser problème pour le code C ++, car la gestion des exceptions est lourde : le cas d'échec est exécuté lentement, l'allocation de mémoire par le cas d'échec (qui parfois n'est même pas disponible) et rend généralement les choses moins prévisibles. La lourdeur d'EH est l'une des raisons pour lesquelles vous entendez des gens dire des choses telles que "n'utilisez pas d'exceptions pour contrôler le flux".

Ainsi, certaines bibliothèques (telles que <filesystem>) utilisent ce que C ++ appelle une "double API" ou ce que C # appelle le Try-Parsemodèle (merci Peter pour le conseil!)

int CalculateArea(int x, int y) {
    if (x < 0 || y < 0) {
        throw Exception("negative side lengths");
    }
    return x * y;
}

bool TryCalculateArea(int x, int y, int& result) {
    if (x < 0 || y < 0) {
        return false;
    }
    result = x * y;
    return true;
}

int a1 = CalculateArea(x, y);
int a2;
if (TryCalculateArea(x, y, a2)) {
    // use a2
}

Vous pouvez voir tout de suite le problème des "doubles API": beaucoup de duplication de code, aucune indication pour les utilisateurs quant à savoir quelle API est la "bonne" à utiliser, et l'utilisateur doit faire un choix difficile entre les messages d'erreur utiles ( CalculateArea) et speed ( TryCalculateArea) parce que la version la plus rapide prend notre "negative side lengths"exception utile et l'aplatit en une inutile false- "quelque chose s'est mal passé, ne me demandez pas quoi ou où". (Certaines API doubles utilisent un type d'erreur plus expressif, tel que int errnoou C ++ std::error_code, mais cela ne vous dit toujours pas l'erreur s'est produite, mais simplement qu'elle s'est produite quelque part.)

Si vous ne pouvez pas décider de la manière dont votre code doit se comporter, vous pouvez toujours lancer la décision à l'appelant!

template<class F>
int CalculateArea(int x, int y, F errorCallback) {
    if (x < 0 || y < 0) {
        return errorCallback(x, y, "negative side lengths");
    }
    return x * y;
}

int a1 = CalculateArea(x, y, [](auto...) { return 0; });
int a2 = CalculateArea(x, y, [](int, int, auto msg) { throw Exception(msg); });
int a3 = CalculateArea(x, y, [](int, int, auto) { return x * y; });

C’est essentiellement ce que fait votre collègue. sauf qu'il factorise le "gestionnaire d'erreur" dans une variable globale:

std::function<int(const char *)> g_errorCallback;

int CalculateArea(int x, int y) {
    if (x < 0 || y < 0) {
        return g_errorCallback("negative side lengths");
    }
    return x * y;
}

g_errorCallback = [](auto) { return 0; };
int a1 = CalculateArea(x, y);
g_errorCallback = [](const char *msg) { throw Exception(msg); };
int a2 = CalculateArea(x, y);

Déplacer des paramètres importants de paramètres de fonction explicites vers un état global est presque toujours une mauvaise idée. Je ne le recommande pas. (Le fait qu'il ne s'agisse pas d' un état global dans votre cas, mais simplement d' un État membre au niveau de l' instance atténue légèrement le problème, mais pas beaucoup.)

De plus, votre collègue limite inutilement le nombre de comportements possibles de gestion des erreurs. Plutôt que de permettre une erreur de traitement des erreurs, il en a décidé deux:

bool g_errorViaException;

int CalculateArea(int x, int y) {
    if (x < 0 || y < 0) {
        return g_errorViaException ? throw Exception("negative side lengths") : 0;
    }
    return x * y;
}

g_errorViaException = false;
int a1 = CalculateArea(x, y);
g_errorViaException = true;
int a2 = CalculateArea(x, y);

C’est probablement le «mauvais côté» de l’une de ces stratégies possibles. Vous avez retiré toute la flexibilité de l'utilisateur final en le forçant à utiliser l'un de vos deux rappels de gestion des erreurs; et vous avez tous les problèmes d'état global partagé; et vous payez toujours pour cette branche conditionnelle partout.

Enfin, une solution commune en C ++ (ou n’importe quel langage avec compilation conditionnelle) consisterait à obliger l’utilisateur à prendre la décision pour l’ensemble du programme, globalement, au moment de la compilation, afin que le chemin de code non utilisé puisse être entièrement optimisé:

int CalculateArea(int x, int y) {
    if (x < 0 || y < 0) {
#ifdef NEXCEPTIONS
        return 0;
#else
        throw Exception("negative side lengths");
#endif
    }
    return x * y;
}

// Now these two function calls *must* have the same behavior,
// which is a nice property for a program to have.
// Improves understandability.
//
int a1 = CalculateArea(x, y);
int a2 = CalculateArea(x, y);

Un exemple de quelque chose qui fonctionne de cette façon est la assertmacro en C et C ++, qui conditionne son comportement sur la macro du préprocesseur NDEBUG.

Quuxplusone
la source
Si l’on retourne un std::optionalde TryCalculateArea(), il est simple d’unifier l’implémentation des deux parties de la double interface dans un seul modèle de fonction avec un indicateur de compilation.
Déduplicateur
@Duplicator: Peut-être avec un std::expected. Avec juste std::optional, à moins que je ne comprenne mal votre solution proposée, elle souffrirait toujours de ce que j’avais dit: l’utilisateur doit faire un choix difficile entre les messages d’erreur utiles et la rapidité, car la version plus rapide prend notre "negative side lengths"exception utile et l’aplatit de manière inutile false- " quelque chose s'est mal passé, ne me demandez pas quoi ou où. "
Quuxplusone le
C'est pourquoi libc ++ <système de fichiers> fait quelque chose de très proche du modèle de collègue d'OP: il std::error_code *ecparcourt tous les niveaux de l'API, puis au fond l'équivalent moral de if (ec == nullptr) throw something; else *ec = some error code. (Cela résume le réel ifen quelque chose appelé ErrorHandler, mais c'est la même idée de base.)
Quuxplusone
Eh bien, ce serait une option pour conserver les informations d'erreur étendues sans les jeter. Peut être approprié, ou ne vaut pas le coût supplémentaire potentiel.
Déduplicateur
1
Tant de bonnes pensées contenues dans cette réponse ... a certainement besoin de plus de votes positifs :-)
cmaster
1

Je pense qu'il faut mentionner d'où votre collègue tire son modèle.

De nos jours, C # a le motif TryGet public bool TryThing(out result). Cela vous permet d'obtenir votre résultat, tout en vous permettant de savoir si ce résultat est même une valeur valide. (Par exemple, toutes les intvaleurs sont des résultats valables pour Math.sum(int, int), mais si la valeur devait déborder, ce résultat particulier pourrait être illégal). C'est un modèle relativement nouveau cependant.

Avant le outmot - clé, vous deviez soit lever une exception (coûteuse, et l'appelant doit l'attraper ou tuer tout le programme), créer une structure spéciale (la classe avant que la classe ou les génériques ne soient vraiment une chose) pour chaque résultat représentant la valeur. et les erreurs possibles (logiciel fastidieux pour créer et gonfler le logiciel), ou renvoyer une valeur "d'erreur" par défaut (qui pourrait ne pas avoir été une erreur).

L’approche que votre collègue utilise donne aux utilisateurs le privilège d’exception lorsqu’ils testent / déboguent de nouvelles fonctionnalités, tout en leur offrant la sécurité et les performances au moment de l’exécution (les performances étaient un problème critique il y a environ 30 ans) consistant à renvoyer une valeur d'erreur par défaut. Il s’agit maintenant du modèle dans lequel le logiciel a été écrit et du modèle attendu, il est donc naturel de continuer ainsi, même s’il existe de meilleures méthodes. Très probablement, ce modèle a été hérité de l’âge du logiciel, ou un modèle que vos collèges n’ont tout simplement pas développé (les vieilles habitudes sont difficiles à rompre).

Les autres réponses expliquent déjà pourquoi cela est considéré comme une mauvaise pratique. Je terminerai donc en vous recommandant de lire le modèle TryGet (peut-être aussi une encapsulation des promesses qu'un objet doit faire à son appelant).

Tezra
la source
Avant le outmot - clé, vous écriviez une boolfonction qui prend un pointeur sur le résultat, c’est-à-dire un refparamètre. Vous pouviez le faire dans VB6 en 1998. Le outmot-clé vous achète simplement une certitude, à la compilation, que le paramètre est attribué au retour de la fonction, c'est tout ce que vous avez à faire. Il est un modèle agréable et utile bien.
Mathieu Guindon le
@MathieuGuindon Yeay, mais GetTry n'était pas encore un modèle bien connu / établi, et même si c'était le cas, je ne suis pas tout à fait sûr qu'il aurait été utilisé. Après tout, une partie de la période qui a précédé l’an 2000 était que le stockage de tout ce qui était plus grand que 0 à 99 était inacceptable.
Tezra
0

Il y a des moments où vous voulez faire son approche, mais je ne considérerais pas que ce soit la situation "normale". La clé pour déterminer dans quel cas vous vous trouvez est la suivante:

Sa logique / raisonnement est que notre programme doit faire une chose, montrer des données à l'utilisateur. Toute autre exception qui ne nous empêche pas de le faire doit être ignorée.

Vérifiez les exigences. Si vos exigences indiquent réellement que vous avez un seul travail, à savoir montrer des données à l'utilisateur, il a raison. Cependant, selon mon expérience, la plupart des fois, l'utilisateur se soucie également des données affichées. Ils veulent les données correctes. Certains systèmes veulent juste échouer discrètement et laisser un utilisateur comprendre que quelque chose ne va pas, mais je les considère comme une exception à la règle.

La question clé que je voudrais poser après une défaillance est la suivante: "Le système est-il dans un état où les attentes de l'utilisateur et les invariants du logiciel sont valides?" Si tel est le cas, alors revenez et continuez. Concrètement, ce n'est pas ce qui se passe dans la plupart des programmes.

En ce qui concerne l'indicateur lui-même, l'indicateur d'exceptions est généralement considéré comme une odeur de code, car un utilisateur doit savoir en quelque sorte le mode du module afin de comprendre le fonctionnement de la fonction. Si elle est en !shouldThrowExceptionsmode, l'utilisateur a besoin de savoir qu'ils sont responsables de la détection des erreurs et de maintenir les attentes et les invariants quand ils se produisent. Ils sont également responsables sur-le-champ, à la ligne où la fonction est appelée. Un drapeau comme celui-ci est généralement très déroutant.

Cependant, cela arrive. Considérez que de nombreux processeurs permettent de modifier le comportement en virgule flottante dans le programme. Un programme qui souhaite des normes plus assouplies peut le faire simplement en changeant un registre (qui est en réalité un drapeau). Le truc est que vous devez être très prudent, pour éviter de marcher accidentellement sur les orteils. Le code vérifie souvent le drapeau actuel, le définit sur le paramètre souhaité, effectue les opérations, puis le réinitialise. De cette façon, personne ne sera surpris par le changement.

Cort Ammon
la source
0

Cet exemple spécifique a une fonctionnalité intéressante qui peut affecter les règles ...

CalculateArea(int x, int y)
{
    if(x < 0 || y < 0)
    {
        if(shouldThrowExceptions) 
            throwException;
        else
            return 0;
    }
}

Ce que je vois ici est un contrôle préalable . Une vérification de précondition qui échoue implique un bogue plus haut dans la pile d'appels. La question est donc de savoir si ce code est responsable de la notification des bugs localisés ailleurs?

Une partie de la tension est due au fait que cette interface présente une obsession primitive - xet yest supposée représenter de vraies mesures de longueur. Dans un contexte de programmation où les types spécifiques de domaines constituent un choix raisonnable, nous rapprocherions en fait de la vérification de la condition préalable de la source des données. meilleur sens pour le contexte.

Cela dit, je ne vois absolument rien qui cloche dans le fait d’avoir deux stratégies différentes pour gérer un chèque en échec. Ma préférence serait d'utiliser la composition pour déterminer quelle stratégie est utilisée; l'indicateur de fonctionnalité serait utilisé dans la racine de la composition, plutôt que dans la mise en œuvre de la méthode de bibliothèque.

// Configurable dependencies
AreaCalculator(PreconditionFailureStrategy strategy)

CalculateArea(int x, int y)
{
    if (x < 0 || y < 0) {
        return this.strategy.fail(0);
    }
    // ...
}

Ils ont travaillé sur des applications critiques de l’aviation où le système ne pouvait pas tomber en panne.

Le National Traffic and Safety Board est vraiment bon; Je pourrais suggérer des techniques d'implémentation alternatives aux graybeards, mais je ne suis pas enclin à discuter avec eux de la conception de cloisons dans le sous-système de rapport d'erreurs.

Plus largement: quel est le coût pour l'entreprise? Casser un site Web coûte beaucoup moins cher qu'un système essentiel à la vie.

VoiceOfUnreason
la source
J'aime la suggestion d'un autre moyen de conserver la flexibilité souhaitée tout en continuant de se moderniser.
drjpizzle
-1

Les méthodes gèrent des exceptions ou ne le font pas, il n’est pas nécessaire d’indicateurs dans des langages comme C #.

public int Method1()
{
  ...code

 return 0;
}

Si quelque chose ne va pas dans ... code, cette exception devra être traitée par l'appelant. Si personne ne gère l'erreur, le programme se terminera.

public int Method1()
{
try {  
...code
}
catch {}
 ...Handle error 
}
return 0;
}

Dans ce cas, si quelque chose de mauvais se produit dans ... code, Method1 gère le problème et le programme doit être exécuté.

C'est à vous de décider où vous gérez les exceptions. Vous pouvez certainement les ignorer en attrapant et en ne faisant rien. Mais, je voudrais m'assurer que vous ignorez seulement certains types d'exceptions spécifiques auxquelles vous pouvez vous attendre. Ignorer ( exception ex) est dangereux car certaines exceptions ne doivent pas être ignorées, comme les exceptions système concernant le manque de mémoire, etc.

Jon Raynor
la source
3
La configuration actuelle publiée par OP concerne la décision de lancer ou non une exception. Le code d'OP ne conduit pas à une ingestion indésirable d'éléments tels que des exceptions de mémoire insuffisante. En tout état de cause, l’affirmation selon laquelle les exceptions font planter le système implique que la base de code ne capture pas les exceptions et n’avalera donc aucune exception; à la fois ceux qui ont été et n'ont pas été jetés intentionnellement par la logique commerciale d'OP.
Flater
-1

Cette approche rompt la philosophie "échec rapide, échec dur".

Pourquoi tu veux échouer vite:

  • Plus vous échouez rapidement, plus le symptôme visible de la défaillance est proche de la cause réelle de celle-ci. Cela facilite beaucoup le débogage - dans le meilleur des cas, vous avez la ligne d'erreur directement dans la première ligne de votre trace de pile.
  • Plus vite vous échouez (et vous attrapez correctement l'erreur), moins vous confondez le reste de votre programme.
  • Plus vous échouez (par exemple, en levant une exception au lieu de simplement renvoyer un code "-1" ou quelque chose du genre), plus il est probable que l'appelant se soucie réellement de l'erreur et ne continue pas à travailler avec des valeurs incorrectes.

Inconvénients de ne pas échouer vite et fort:

  • Si vous évitez les échecs visibles, c’est-à-dire que tout va bien, vous aurez alors beaucoup de mal à trouver l’erreur réelle. Imaginez que la valeur de retour de votre exemple fasse partie d’une routine qui calcule la somme de 100 zones; C'est-à-dire que vous appelez cette fonction 100 fois et que vous additionnez les valeurs de retour. Si vous supprimez l’erreur en silence, il n’ya aucun moyen de trouver où l’erreur réelle se produit; et tous les calculs suivants seront silencieusement faux.
  • Si vous tardez à échouer (en renvoyant une valeur de retour impossible telle que "-1" pour une zone), vous augmentez la probabilité que l'appelant de votre fonction ne se soucie plus de rien et oublie de gérer l'erreur; même s'ils ont les informations sur l'échec sous la main.

Enfin, la gestion des erreurs basée sur les exceptions présente l’avantage de pouvoir donner un message ou un objet "hors bande", vous pouvez facilement vous connecter à la journalisation des erreurs, aux alertes, etc. sans écrire une seule ligne supplémentaire dans votre code de domaine. .

Il existe donc non seulement des raisons techniques simples, mais également des raisons "système" qui rendent très utile l’échec rapide.

À la fin de la journée, ne pas rater le plus vite possible à l’heure actuelle, où la gestion des exceptions est légère et très stable est à moitié criminelle. Je comprends parfaitement pourquoi il est bon de penser qu’il est bon de supprimer les exceptions, mais cela n’est tout simplement plus applicable.

Surtout dans votre cas particulier, où vous donnez même la possibilité de lancer ou non des exceptions: cela signifie que l'appelant doit décider de toute façon . Il n’ya donc aucun inconvénient à ce que l’appelant attrape l’exception et la traite de manière appropriée.

Un point à venir dans un commentaire:

Il a été souligné dans d'autres réponses qu'il ne serait pas souhaitable d'échouer rapidement et difficilement dans une application critique lorsque le matériel utilisé est un avion.

Un échec rapide et difficile ne signifie pas que toute votre application se bloque. Cela signifie que l'erreur se produit localement au moment où l'erreur se produit. Dans l'exemple du PO, la méthode de bas niveau qui calcule une zone ne doit pas remplacer silencieusement une erreur par une valeur erronée. Cela devrait clairement échouer.

Certains appelants de la chaîne doivent évidemment détecter cette erreur / exception et la gérer de manière appropriée. Si cette méthode était utilisée dans un avion, cela devrait probablement entraîner l’allumage d’une LED d’erreur, ou tout au moins afficher «une zone de calcul d’erreur» au lieu d’une zone erronée.

AnoE
la source
3
Dans d'autres réponses, il a été souligné qu'il ne serait pas souhaitable d'échouer rapidement et difficilement dans une application critique lorsque le matériel utilisé est un avion.
HAEM
1
@HAEM, alors c'est un malentendu sur ce que signifie échouer rapidement et difficilement. J'ai ajouté un paragraphe à ce sujet à la réponse.
AnoE
Même si l'intention n'est pas d'échouer «aussi dur», il est toujours considéré comme risqué de jouer avec ce type de feu.
drjpizzle
C'est un peu le but, @drjpizzle. Si vous êtes habitué à échouer rapidement, il n’est pas "risqué" ou "de jouer avec ce genre de feu" dans mon expérience. Au contraire. Cela signifie que vous êtes habitué à penser à "que se passerait-il si je recevais une exception ici", où "ici" signifie partout , et vous avez tendance à savoir si l'endroit où vous programmez actuellement posera un problème majeur ( accident d'avion, peu importe dans ce cas). Jouer avec le feu, ce serait s’attendre à ce que tout se passe normalement, et que chaque élément, en cas de doute, prétend que tout va bien ...
AnoE