Pourquoi les blocs de capture vides sont-ils une mauvaise idée? [fermé]

Je viens de voir une question sur try-catch , quelles personnes (y compris Jon Skeet) disent que les blocs catch vides sont une très mauvaise idée? Pourquoi ça? N'y a-t-il pas de situation où une prise vide n'est pas une mauvaise décision de conception?

Je veux dire, par exemple, parfois, vous voulez obtenir des informations supplémentaires de quelque part (service Web, base de données) et vous ne vous souciez vraiment pas de savoir si vous obtiendrez ces informations ou non. Donc vous essayez de l'obtenir, et si quelque chose arrive, c'est ok, je vais juste ajouter un "catch (Exception ignored) {}" et c'est tout

Habituellement, un try-catch vide est une mauvaise idée car vous avalez silencieusement une condition d'erreur et continuez ensuite l'exécution. Parfois, cela peut être la bonne chose à faire, mais c'est souvent le signe qu'un développeur a vu une exception, ne savait pas quoi faire à ce sujet et a donc utilisé une capture vide pour faire taire le problème.

C'est l'équivalent de programmation de mettre du ruban noir sur un voyant d'avertissement du moteur.

Je crois que la façon dont vous gérez les exceptions dépend de la couche du logiciel dans laquelle vous travaillez: Exceptions in the Rainforest .

C'est une mauvaise idée en général car c'est une condition vraiment rare où une panne (condition exceptionnelle, plus génériquement) est correctement rencontrée sans aucune réponse. En plus de cela, les catchblocs vides sont un outil couramment utilisé par les personnes qui utilisent le moteur d'exception pour vérifier les erreurs qu'elles devraient faire de manière préventive.

Dire que c'est toujours mauvais est faux ... c'est très peu vrai. Il peut y avoir des circonstances où vous ne vous souciez pas qu'il y ait eu une erreur ou que la présence de l'erreur indique d'une manière ou d'une autre que vous ne pouvez rien y faire de toute façon (par exemple, lors de l'écriture d'une erreur précédente dans un fichier journal texte et vous obtenez un IOException, ce qui signifie que vous ne pouvez pas écrire la nouvelle erreur de toute façon).

Je n'irais pas jusqu'à dire que celui qui utilise des blocs catch vides est un mauvais programmeur et ne sait pas ce qu'il fait ...

J'utilise des blocs catch vides si nécessaire. Parfois, le programmeur de la bibliothèque que je consomme ne sait pas ce qu'il fait et lance des exceptions même dans des situations où personne n'en a besoin.

Par exemple, considérez une bibliothèque de serveur http, je m'en fiche si le serveur lève une exception car le client s'est déconnecté et index.htmln'a pas pu être envoyé.

Il existe de rares cas où cela peut être justifié. En Python, vous voyez souvent ce type de construction:

try:
    result = foo()
except ValueError:
    result = None

Il est donc possible (selon votre application) de faire:

result = bar()
if result == None:
    try:
        result = foo()
    except ValueError:
        pass # Python pass is equivalent to { } in curly-brace languages
 # Now result == None if bar() returned None *and* foo() failed

Dans un récent projet .NET, j'ai dû écrire du code pour énumérer les DLL de plug-in afin de trouver des classes qui implémentent une certaine interface. Le bit de code pertinent (dans VB.NET, désolé) est:

    For Each dllFile As String In dllFiles
        Try
            ' Try to load the DLL as a .NET Assembly
            Dim dll As Assembly = Assembly.LoadFile(dllFile)
            ' Loop through the classes in the DLL
            For Each cls As Type In dll.GetExportedTypes()
                ' Does this class implement the interface?
                If interfaceType.IsAssignableFrom(cls) Then

                    ' ... more code here ...

                End If
            Next
        Catch ex As Exception
            ' Unable to load the Assembly or enumerate types -- just ignore
        End Try
    Next

Bien que même dans ce cas, j'admets que la journalisation de l'échec quelque part serait probablement une amélioration.

Des blocs de capture vides sont généralement placés parce que le codeur ne sait pas vraiment ce qu'il fait. Dans mon organisation, un bloc catch vide doit inclure un commentaire expliquant pourquoi ne rien faire à l'exception est une bonne idée.

Dans le même ordre d'idées, la plupart des gens ne savent pas qu'un bloc try {} peut être suivi par un catch {} ou un finally {}, un seul est requis.

Les exceptions ne devraient être levées que s'il y a vraiment une exception - quelque chose qui se passe au-delà de la norme. Un bloc de capture vide dit essentiellement "quelque chose de mauvais se passe, mais je m'en fiche". C'est une mauvaise idée.

Si vous ne souhaitez pas gérer l'exception, laissez-la se propager vers le haut jusqu'à ce qu'elle atteigne un code capable de la gérer. Si rien ne peut gérer l'exception, l'application doit être supprimée.

Je pense que ce n'est pas grave si vous attrapez un type d'exception particulier dont vous savez qu'il ne sera déclenché que pour une raison particulière , et vous vous attendez à cette exception et vous n'avez vraiment rien à faire à ce sujet.

Mais même dans ce cas, un message de débogage pourrait être en ordre.

Par Josh Bloch - Point 65: N'ignorez pas les exceptions de Java efficace :

1. Un bloc catch vide va à l'encontre du but des exceptions
2. À tout le moins, le bloc catch devrait contenir un commentaire expliquant pourquoi il est approprié d'ignorer l'exception.

Un bloc catch vide dit essentiellement "Je ne veux pas savoir quelles erreurs sont lancées, je vais juste les ignorer."

C'est similaire à VB6 On Error Resume Next, sauf que tout ce qui se trouve dans le bloc try après le lancement de l'exception sera ignoré.

Ce qui n'aide pas quand quelque chose se brise.

Cela va de pair avec «N'utilisez pas d'exceptions pour contrôler le déroulement du programme» et «N'utilisez les exceptions que dans des circonstances exceptionnelles». Si cela est fait, les exceptions ne devraient se produire qu'en cas de problème. Et s'il y a un problème, vous ne voulez pas échouer en silence. Dans les rares anomalies où il n'est pas nécessaire de gérer le problème, vous devez au moins enregistrer l'exception, juste au cas où l'anomalie ne deviendrait plus une anomalie. La seule chose pire que l'échec est l'échec en silence.

Je pense qu'un bloc catch complètement vide est une mauvaise idée car il n'y a aucun moyen de déduire qu'ignorer l'exception était le comportement prévu du code. Il n'est pas forcément mal d'avaler une exception et de renvoyer false ou null ou une autre valeur dans certains cas. Le framework .net a de nombreuses méthodes "try" qui se comportent de cette façon. En règle générale, si vous avalez une exception, ajoutez un commentaire et une instruction de journal si l'application prend en charge la journalisation.

Parce que si une exception est lancée, vous ne la verrez jamais - échouer silencieusement est la pire option possible - vous obtiendrez un comportement erroné et aucune idée de regarder où cela se passe. Mettez au moins un message de journal là-bas! Même si c'est quelque chose qui «ne peut jamais arriver»!

Les blocs catch vides indiquent qu'un programmeur ne sait pas quoi faire avec une exception. Ils suppriment l'exception qui risque de bouillonner et d'être gérée correctement par un autre bloc try. Essayez toujours de faire quelque chose, sauf que vous attrapez.

Je trouve le plus ennuyeux avec les déclarations catch vides, c'est quand un autre programmeur l'a fait. Ce que je veux dire, c'est que lorsque vous avez besoin de déboguer le code de quelqu'un d'autre, toute instruction catch vide rend une telle entreprise plus difficile qu'elle ne doit l'être. Les déclarations de capture IMHO doivent toujours afficher une sorte de message d'erreur - même si l'erreur n'est pas gérée, elle doit au moins la détecter (alt. Activée uniquement en mode débogage)

Ce n'est probablement jamais la bonne chose car vous passez silencieusement toutes les exceptions possibles. S'il y a une exception spécifique que vous attendez, vous devez la tester, la renvoyer si ce n'est pas votre exception.

try
{
    // Do some processing.
}
catch (FileNotFound fnf)
{
    HandleFileNotFound(fnf);
}
catch (Exception e)
{
    if (!IsGenericButExpected(e))
        throw;
}

public bool IsGenericButExpected(Exception exception)
{
    var expected = false;
    if (exception.Message == "some expected message")
    {
        // Handle gracefully ... ie. log or something.
        expected = true;
    }

    return expected;
}

En règle générale, vous ne devez détecter que les exceptions que vous pouvez réellement gérer. Cela signifie être aussi précis que possible lors de la détection des exceptions. Attraper toutes les exceptions est rarement une bonne idée et ignorer toutes les exceptions est presque toujours une très mauvaise idée.

Je ne peux penser qu'à quelques exemples où un bloc catch vide a un but significatif. Si une exception spécifique que vous attrapez est "gérée" en réessayant simplement l'action, il n'y aurait pas besoin de faire quoi que ce soit dans le bloc catch. Cependant, il serait toujours judicieux de consigner le fait que l'exception s'est produite.

Autre exemple: CLR 2.0 a changé la façon dont les exceptions non gérées sur le thread du finaliseur sont traitées. Avant la version 2.0, le processus était autorisé à survivre à ce scénario. Dans le CLR actuel, le processus est arrêté en cas d'exception non gérée sur le thread du finaliseur.

Gardez à l'esprit que vous ne devez implémenter un finaliseur que si vous en avez vraiment besoin et même dans ce cas, vous devez en faire le moins possible dans le finaliseur. Mais si le travail que votre finaliseur doit faire peut générer une exception, vous devez choisir entre le moindre de deux maux. Voulez-vous arrêter l'application en raison de l'exception non gérée? Ou souhaitez-vous procéder dans un état plus ou moins indéfini? Au moins en théorie, ce dernier peut être le moindre de deux maux dans certains cas. Dans ce cas, le bloc catch vide empêcherait le processus de se terminer.

Je veux dire, par exemple, parfois, vous voulez obtenir des informations supplémentaires de quelque part (service Web, base de données) et vous ne vous souciez vraiment pas de savoir si vous obtiendrez ces informations ou non. Donc vous essayez de l'obtenir, et si quelque chose arrive, c'est ok, je vais juste ajouter un "catch (Exception ignored) {}" et c'est tout

Donc, pour reprendre votre exemple, c'est une mauvaise idée dans ce cas parce que vous attrapez et ignorez toutes les exceptions. Si vous attrapiez seulement EInfoFromIrrelevantSourceNotAvailableet l'ignoriez, ce serait bien, mais vous ne l'êtes pas. Vous ignorez également ENetworkIsDown, ce qui peut être important ou non. Vous ignorez ENetworkCardHasMeltedet EFPUHasDecidedThatOnePlusOneIsSeventeen, qui sont presque certainement importants.

Un bloc catch vide n'est pas un problème s'il est configuré pour attraper (et ignorer) uniquement les exceptions de certains types que vous savez être sans importance. Les situations dans lesquelles c'est une bonne idée de supprimer et d'ignorer silencieusement toutes les exceptions, sans s'arrêter pour les examiner d'abord pour voir si elles sont attendues / normales / non pertinentes ou non, sont extrêmement rares.

Il y a des situations où vous pourriez les utiliser, mais elles devraient être très rares. Les situations dans lesquelles je pourrais en utiliser un incluent:

• journalisation des exceptions; selon le contexte, vous souhaiterez peut-être qu'une exception non gérée ou un message soit publié à la place.

• des situations techniques en boucle, comme le rendu ou le traitement du son ou un rappel de listbox, où le comportement lui-même montrera le problème, le fait de lancer une exception ne fera que gêner et la journalisation de l'exception entraînera probablement des milliers de messages "échoué à XXX" .

• programmes qui ne peuvent pas échouer, même s'ils devraient au moins enregistrer quelque chose.

pour la plupart des applications winforms, j'ai trouvé qu'il suffit d'avoir une seule instruction try pour chaque entrée utilisateur. J'utilise les méthodes suivantes: (AlertBox est juste un wrapper MessageBox.Show rapide)

  public static bool TryAction(Action pAction)
  {
     try { pAction(); return true; }
     catch (Exception exception)
     {
        LogException(exception);
        return false;
     }
  }

  public static bool TryActionQuietly(Action pAction)
  {
     try { pAction(); return true; }
     catch(Exception exception)
     {
        LogExceptionQuietly(exception);
        return false;
     }
  }

  public static void LogException(Exception pException)
  {
     try
     {
        AlertBox(pException, true);
        LogExceptionQuietly(pException);
     }
     catch { }
  }

  public static void LogExceptionQuietly(Exception pException)
  {
     try { Debug.WriteLine("Exception: {0}", pException.Message); } catch { }
  }

Ensuite, chaque gestionnaire d'événements peut faire quelque chose comme:

  private void mCloseToolStripMenuItem_Click(object pSender, EventArgs pEventArgs)
  {
     EditorDefines.TryAction(Dispose);
  }

ou

  private void MainForm_Paint(object pSender, PaintEventArgs pEventArgs)
  {
     EditorDefines.TryActionQuietly(() => Render(pEventArgs));
  }

Théoriquement, vous pourriez avoir TryActionSilently, ce qui pourrait être mieux pour le rendu des appels afin qu'une exception ne génère pas une quantité infinie de messages.

Si vous ne savez pas quoi faire dans catch block, vous pouvez simplement enregistrer cette exception, mais ne la laissez pas vide.

        try
        {
            string a = "125";
            int b = int.Parse(a);
        }
        catch (Exception ex)
        {
            Log.LogError(ex);
        }

Vous ne devriez jamais avoir un bloc catch vide. C'est comme cacher une erreur que vous connaissez. À tout le moins, vous devriez écrire une exception dans un fichier journal pour la revoir plus tard si vous êtes pressé par le temps.