"À quel point" est un code sans rapport avec le bloc try-catch-finally?

Ceci est une question connexe Q: Est-ce que l'utilisation de la clause finally pour effectuer un travail après un mauvais style de retour est dangereuse?

Dans le Q référencé, le code finalement est lié à la structure utilisée et à la nécessité de la prélecture. Ma question est un peu différente, et je pense qu'elle est pertinente pour un public plus large. Mon exemple particulier est une application Winform C #, mais cela s'appliquerait également à l'utilisation C ++ / Java.

Je remarque pas mal de blocs try-catch-finally où il y a beaucoup de code sans rapport avec les exceptions et la gestion / nettoyage des exceptions enfouies dans le bloc. Et j'admettrai mon parti pris pour avoir des blocs try-catch-finally très serrés avec le code étroitement lié à l'exception et à la gestion. Voici quelques exemples de ce que je vois.

Les blocs try auront beaucoup d'appels et de variables préliminaires définis menant au code qui pourrait être lancé. Les informations de journalisation seront également configurées et exécutées dans le bloc try.

Enfin, les blocs auront des appels de formatage / module / contrôle (bien que l'application soit sur le point de se terminer, comme indiqué dans le bloc catch), ainsi que la création de nouveaux objets tels que des panneaux.

Grossièrement:

    methodName (...)
    {
        essayer
        {
            // Beaucoup de code pour la méthode ...
            // code qui pourrait lancer ...
            // Beaucoup plus de code pour la méthode et un retour ...
        }
        attraper (quelque chose)
        {// gérer l'exception}
        enfin
        {
            // un nettoyage dû à une exception, fermant les choses
            // plus de code pour le contenu créé (en ignorant que des exceptions auraient pu être levées) ...
            // peut-être créer d'autres objets
        }
    }

Le code fonctionne, il a donc une certaine valeur. Il n'est pas bien encapsulé et la logique est un peu compliquée. Je suis (douloureusement) familier avec les risques liés au changement de code et à la refactorisation, donc ma question se résume à vouloir connaître l'expérience des autres avec un code de structure similaire.

Le mauvais style justifie-t-il les changements? Quelqu'un a-t-il été gravement brûlé d'une situation similaire? Souhaitez-vous partager les détails de cette mauvaise expérience? Laissez-le parce que je réagis de manière excessive et ce n'est pas si mal que ça de style? Bénéficier des avantages de l'entretien de ranger les choses?

J'ai vécu une situation très similaire lorsque j'ai dû faire face à un terrible code Windows Forms hérité écrit par des développeurs qui ne savaient clairement pas ce qu'ils faisaient.

Tout d'abord, vous n'agissez pas trop. C'est du mauvais code. Comme vous l'avez dit, le bloc de capture devrait concerner l'interruption et la préparation de l'arrêt. Ce n'est pas le moment de créer des objets (spécialement des panneaux). Je ne peux même pas commencer à expliquer pourquoi c'est mauvais.

Cela étant dit...

Mon premier conseil est: s'il n'est pas cassé, ne le touchez pas!

Si votre travail consiste à maintenir le code, vous devez faire de votre mieux pour ne pas le casser. Je sais que c'est douloureux (j'y suis allé) mais vous devez faire de votre mieux pour ne pas casser ce qui fonctionne déjà.

Mon deuxième conseil est: si vous devez ajouter plus de fonctionnalités, essayez de conserver la structure de code existante autant que possible afin de ne pas casser le code.

Exemple: s'il y a une déclaration hideuse de cas de commutation qui, selon vous, pourrait être remplacée par un héritage approprié, vous devez être prudent et réfléchir à deux fois avant de décider de commencer à déplacer les choses.

Vous trouverez certainement des situations où un refactoring est la bonne approche mais attention: le code de refactoring est plus susceptible d'introduire des bugs . Vous devez prendre cette décision du point de vue des propriétaires d'applications, et non du point de vue du développeur. Vous devez donc penser si l'effort (argent) nécessaire pour résoudre le problème vaut la peine d'être refactorisé ou non. J'ai vu plusieurs fois un développeur passer plusieurs jours à réparer quelque chose qui n'est pas vraiment cassé simplement parce qu'il pense que "le code est moche".

Mon troisième conseil est: vous vous brûlerez si vous cassez le code, peu importe que ce soit de votre faute ou non.

Si vous avez été embauché pour effectuer la maintenance, peu importe si la demande s'effondre parce que quelqu'un d'autre a pris de mauvaises décisions. Du point de vue de l'utilisateur, cela fonctionnait auparavant et maintenant vous l'avez cassé. Tu l'as cassé!

Joel met très bien dans son article expliquant plusieurs raisons pour lesquelles vous ne devriez pas réécrire le code hérité.

http://www.joelonsoftware.com/articles/fog0000000069.html

Donc, vous devriez vous sentir vraiment mal à propos de ce type de code (et vous ne devriez jamais écrire quelque chose comme ça) mais le maintenir est un tout autre monstre.

À propos de mon expérience: j'ai dû maintenir le code pendant environ 1 an et j'ai finalement pu le réécrire à partir de zéro, mais pas en une seule fois.

Ce qui s'est passé, c'est que le code était si mauvais que de nouvelles fonctionnalités étaient impossibles à implémenter. L'application existante présentait de sérieux problèmes de performances et de convivialité. Finalement, on m'a demandé de faire un changement qui me prendrait 3-4 mois (principalement parce que travailler avec ce code m'a pris beaucoup plus de temps que d'habitude). J'ai pensé que je pourrais réécrire cette pièce entière (y compris la mise en œuvre de la nouvelle fonctionnalité souhaitée) dans environ 5-6 mois. J'ai apporté cette proposition aux parties prenantes et elles acceptent de la réécrire (heureusement pour moi).

Après avoir réécrit cette pièce, ils ont compris que je pouvais livrer des choses bien meilleures que ce qu'ils avaient déjà. J'ai donc pu réécrire toute l'application.

Mon approche était de le réécrire morceau par morceau. J'ai d'abord remplacé l'intégralité de l'interface utilisateur (Windows Forms), puis j'ai commencé à remplacer la couche de communication (appels de service Web) et enfin j'ai remplacé l'intégralité de l'implémentation du serveur (c'était une sorte d'application client / serveur épaisse).

Quelques années plus tard, cette application est devenue un bel outil clé utilisé par toute l'entreprise. Je suis sûr à 100% que cela n'aurait jamais été possible si je n'avais pas réécrit le tout.

Même si j'ai pu le faire, l'important est que les parties prenantes l'ont approuvé et j'ai pu les convaincre que cela en valait la peine. Donc, bien que vous deviez maintenir l'application existante, faites de votre mieux pour ne rien casser et si vous êtes en mesure de convaincre les propriétaires des avantages de la réécrire, essayez de le faire comme Jack l'Éventreur: par morceaux.

S'il n'est pas nécessaire de modifier le code, laissez-le tel quel. Mais si vous devez changer le code parce que vous devez corriger un bogue ou changer certaines fonctionnalités, vous devrez le changer, que cela vous plaise ou non. À mon humble avis, il est souvent plus sûr et moins sujet aux erreurs, si vous le refactorisez d'abord en morceaux plus petits et mieux compréhensibles, puis ajoutez la fonctionnalité. Lorsque vous avez des outils de refactoring automatiques à portée de main, cela peut être fait relativement en toute sécurité, avec seulement un très petit risque d'introduire de nouveaux bogues. Et je vous recommande de vous procurer une copie du livre de Michael Feathers

http://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052

ce qui vous donnera des conseils précieux sur la façon de rendre le code plus testable, d'ajouter des tests unitaires et d'éviter de casser le code lors de l'ajout de nouvelles fonctionnalités.

Si vous le faites correctement, votre méthode deviendra, espérons-le, assez simple, le try-catch-enfin ne contiendra plus de code sans rapport aux mauvais endroits.

Supposons que vous ayez six méthodes à appeler, dont quatre ne doivent être appelées que si la première se termine sans erreur. Considérez deux alternatives:

try {
     method1();  // throws
     method2();
     method3();
     method4();
     method5();
 } catch(e) {
     // handle error
 }
 method6();

contre

 try {
      method1();  // throws
 }
 catch(e) {
      // handle error
 }
 if(! error ) {
     method2();
     method3();
     method4();
     method5();
 }
 method6();

Dans le second code, vous traitez les exceptions comme les codes retour. Conceptuellement, cela est identique à:

rc = method1();
if( rc != error ) {
     method2();
     method3();
     method4();
     method5();
 }
 method6();

Si nous allons encapsuler toutes les méthodes qui peuvent lever une exception dans un bloc try / catch, quel est l'intérêt d'avoir des exceptions dans une fonctionnalité de langage? Il n'y a aucun avantage et il y a plus de frappe.

La raison pour laquelle nous avons des exceptions dans les langues en premier lieu est que dans le mauvais vieux temps des codes de retour, nous avions souvent des situations ci-dessus où nous avions plusieurs méthodes qui pouvaient renvoyer des erreurs, et chaque erreur signifiait abandonner toutes les méthodes entièrement. Au début de ma carrière, dans l'ancien temps C, j'ai vu du code comme ça partout:

rc = method1();
if( rc != error ) {
     rc = method2();
     if( rc != error ) {
         rc = method3();
         if( rc != error ) {
             rc = method4();
             if(rc != error ) {
                 method5();
             }
         }
     }
 }
 method6();

C'était un modèle incroyablement commun, et celui que les exceptions ont très bien résolu en vous permettant de revenir au premier exemple ci-dessus. Une règle de style qui dit que chaque méthode a son propre bloc d'exceptions jette complètement cela par la fenêtre. Pourquoi même déranger, alors?

Vous devez toujours vous efforcer de faire en sorte que le bloc try soit entouré de l'ensemble de code qui est conceptuellement une unité. Il devrait entourer le code pour lequel s'il y a une erreur dans l'unité de code, alors il n'y a aucun intérêt à exécuter un autre code dans l'unité de code.

Pour en revenir à l'objectif initial des exceptions: rappelez-vous qu'elles ont été créées car, souvent, le code ne peut pas facilement traiter les erreurs au moment même où elles se produisent. Le point des exceptions est de vous permettre de traiter les erreurs beaucoup plus tard dans le code, ou plus loin dans la pile. Les gens oublient cela et, dans de nombreux cas, les attrapent localement quand ils auraient intérêt à simplement documenter que la méthode en question peut lever cette exception. Il y a trop de code dans le monde qui ressemble à ceci:

void method0() : throws MyNewException 
{
    try {
        method1();  // throws MyOtherException
    }
    catch(e) {
        if(e == MyOtherException)
            throw MyNewException();
    }
    method2();
}

Ici, vous ajoutez simplement des calques, et les calques ajoutent de la confusion. Faites ceci:

void method0() : throws MyOtherException
{
    method1();
    method2();
}

En plus de cela, mon sentiment est que si vous suivez cette règle et que vous vous trouvez avec plus d'un couple de blocs try / catch dans la méthode, vous devez vous demander si la méthode elle-même est trop complexe et doit être décomposée en plusieurs méthodes .

Le point à retenir: un bloc try doit contenir l'ensemble de code qui doit être abandonné si une erreur se produit n'importe où dans le bloc. Que cet ensemble de code soit une ligne ou mille lignes n'a pas d'importance. (Bien que si vous avez mille lignes dans une méthode, vous avez d'autres problèmes.)