Levée intentionnelle d'exceptions à l'utilisation des captures

10

Pour un exemple typique if...elsede gestion des exceptions avec encapsulation, l'exemple suivant est-il une pratique recommandée pour éviter la duplication de code?

try
{
    if (GetDataFromServer())
    {
        return ProcessData();
    }
    else
    {
        throw new Exception();
    }
catch(Exception ex)
{
    return null;
}

au lieu de...

try
{
    if (GetDataFromServer())
    {
        return ProcessData();
    }
    else
    {
        return null;
    }
}
catch(Exception ex)
{
    return null;
}

Je sais qu'il y a une légère baisse de performances, mais je me demande si cela est considéré comme une pratique acceptable. Je fais actuellement la deuxième méthode - en particulier dans les cas où j'ai besoin de gérer des exceptions spécifiques différemment - mais je me demandais si la première méthode est appropriée pour les cas simples.

grovesNL
la source
si la méthode est suffisamment petite, je supprimerai juste le else et retournerai null en dehors du bloc trycatch donc je ne dois retourner null qu'une seule fois.
Fabio Marcolini

Réponses:

12

L'utilisation de la gestion des exceptions pour le contrôle de flux est déconseillée par Microsoft.

Et une table ronde sur le sujet est disponible.

Cela étant dit, C # prend en charge cela, et je suppose que cela dépend de la condition rencontrée si une exception est la réponse la plus appropriée.

B2K
la source
1
Il me semble que ce genre de chose essaie vraiment de ne pas utiliser les événements.
radarbob
@radarbob: Comment les événements sont-ils liés à cela?
@grovesNL - Lancer une exception à un point spécifique pour appeler une certaine méthode dans un bloc catch? Des charlatans comme un événement pour moi.
radarbob
@radarbob: Ce n'est pas un événement. Il y a beaucoup d'exemples d'utilisation où cela serait utilisé, comme indiqué dans le lien de la table ronde de la réponse.
1
@radarbob Juste une clarification, les exceptions ont été conçues comme un moyen de signaler à l'appelant que quelque chose s'est produit que la méthode appelée n'est pas en mesure de gérer. Cependant, un événement doit être écouté. Une exception est une interruption forcée du déroulement normal d'un programme. Une exception non interceptée entraînera la fin anormale de l'application entière.
6

Le hit de performance est très probablement négligeable, comme expliqué dans cette réponse .

Partons donc avec l'idée que la performance n'est pas un problème. Vous lancez System.Exception, juste pour déplacer l'exécution dans la catchclause . Jeter un BadControlFlowThatShouldBeRewrittenExceptionserait probablement exagéré.

Décomposons cela. On a:

  • Méthode GetDataFromServer(les noms de méthode doivent être PascalCase en C #), qui peut éventuellement lever une exception ou renvoyer un bool.
  • Si le résultat était true, exécutez ProcessData.
  • Revenez nullautrement.

On dirait que la méthode où ce code est écrit, fait simplement trop de choses. GetDataFromServerrenvoyer un boolressemble à un défaut de conception, je m'attendrais à ce que cette méthode retourne les données qu'elle obtient du serveur , certaines IEnumerable<SomeType>qui contiendraient 0 ou plusieurs éléments - c'est-à-dire que le chemin heureux renvoie n éléments où n> 0 , pas si heureux path retourne 0 élément, et unhappy path explose avec une exception non gérée, quelle qu'elle soit.

Cela change beaucoup l'apparence de la méthode - encore une fois, il est difficile de dire si cela a du sens, car la publication d'origine n'a qu'un seul point de sortie (et donc ne compilerait pas, car tous les chemins de code ne renvoient pas de valeur ), donc ce n'est qu'une supposition folle:

try
{
    var result = GetDataFromServer();
    return ProcessData(result);
}
catch
{
    return null;
}

Ici, vous regarderiez ProcessDataet verriez qu'il est en train d'itérer le resultet retourne nulls'il n'y a pas d'article dans le IEnumerable.

Maintenant, pourquoi la méthode revient-elle null? Le serveur était en panne? Y a-t-il un bug dans la requête? La chaîne de connexion utilise des informations d'identification incorrectes? Chaque fois que vous GetDataFromServerexplosez avec une exception à laquelle vous ne vous attendez pas, vous l'avalez, la fourrez sous le tapis et retournez une nullvaleur. Je recommande d'attraper des exceptions spécifiques dans ce cas et de consigner tout le reste; le débogage sera beaucoup plus facile de cette façon.

Avec une catchclause générale qui ne capture pas l'exception, il devient assez difficile de diagnostiquer quoi que ce soit. Je ferais le moins possible à la place:

catch(Exception e)
{
    return null;
}

Maintenant, vous pouvez au moins casser et inspecter esi les choses tournent mal.


TL; DR : Non, lever et intercepter des exceptions pour le contrôle de flux n'est pas une bonne idée.

Mathieu Guindon
la source
Cette réponse montre exactement pourquoi j'ai essayé de garder mon code générique: je ne voulais pas énumérer toutes les exceptions que je fais réellement dans mon code; Je ne voulais pas lister les noms de méthode réels; Je ne voulais pas lister la déclaration de méthode; Je ne voulais pas de suggestions de syntaxe. J'avais une seule question à savoir si lever des exceptions pour le contrôle de flux était correct, ce à quoi B2K a rapidement répondu. Je serais heureux d'en débattre sur les méta.
3
sonne comme cela aurait dû être une question pour les programmeurs alors. nous examinons le code, pas les idées.
Malachi
2

dans votre première réponse, il y a un impact sur les performances qui n'a pas besoin d'être là.

try
{
    if (GetDataFromServer())
    {
        return ProcessData();
    }
    else
    {
        throw new Exception();
    }
catch(Exception ex)
{
    return null;
}

lorsque vous quittez l'instruction if pour entrer dans l'instruction Catch alors que vous n'avez pas à faire changer de direction, pour ainsi dire.

si vous voulez le return null; faire dans l'instruction else et non dans une capture qui est interceptée après avoir été levée à partir de l'instruction else.

Ne s'applique probablement pas à votre code réel , mais pour le code générique que vous avez donné, il s'applique.

Les normes disent que vous ne devriez pas faire ça.

Les normes disent que vous devriez le faire comme ça, (encore une fois basé sur le code générique donné dans OP)

if (GetDataFromServer())
{
    return ProcessData();
}
else
{
    Return null
}

et puisque vous n'avez aucune exception spécifique que vous attrapez, vous ne devriez même pas avoir de prise d'essai ici.

vous souhaitez voir les exceptions lorsqu'elles se produisent afin de pouvoir résoudre le problème qui crée l'exception.

Malachie
la source
1

Pourquoi pas beaucoup plus simple:

if (!GetDataFromServer()) return null;
ProcessData();

Si un gestionnaire d'exceptions doit exister, il doit être dans ProcessData ()

Loren Pechtel
la source
Pourquoi ne voudrais-je pas passer des exceptions ProcessData()au niveau supérieur?
grovesNL
@grovesNL Rien d'utile n'était fait à l'exception ici.
Loren Pechtel
1
Comment? Si ProcessData()lève une exception maintenant, elle n'est pas gérée. Je le veux return nullà ce niveau si ProcessData()jette une exception, sans se modifier ProcessData().
grovesNL du