Demande d'avis sur bout de code

J'interviens sur un projet ayant connu plusieurs développeur, et je tombe sur un process de gestion d'erreur, liée au traitement de message, correspondant toujours au même template... qui me tire un peu l'oeil.
Quelqu'un aurait un avis dessus?

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18

} catch (Exception e) { String strCode = null; String strMsg = null; if (e.getClass().getName().endsWith("MyCorpException")) { strCode = ((MyCorpException) e).getstrCode(); strMsg = ((MyCorpException) e).getLibelleErreur(); } else { strCode = "ERR_TRT_N3"; strMsg = "erreur general sur non traitable."; + message.getProperties().getProperty(CstCode.PROP1); } message.getProperties().setProperty(CstCode.PROP_CDE_ERREUR, strCode); message.getProperties().setProperty(CstCode.PROP_LIB_ERREUR, strMsg); }   return message; }

---

On a pas beaucoup de détails sur le code ou le contexte.
Mais bon...
Pour moi c'est très moche.

Le catch prend toutes les Exceptions, et différencie celles finissant par "MyCorpException" des autres. Erk...

Donc en plus du catch(Exception) qui en soit est généralement à éviter, sauf très bonne raison, je pense qu'il aurait été plus propre de faire une hiérarchie d'exception genre :

Code:

---

1 2 3

class MyCorpException extends Exception { ... } class MySubCorpException1 extends MyCorpException { ... } class MySubCorpException2 extends MyCorpException { ... }

---

Puis, pour traiter les deux cas de figures comme dans le code initial, faire :

Code:

---

1 2 3 4 5

} catch (MyCorpException e) { ... } catch (Exception e) { ... }

---

Pour le fond, je pense que sans plus d'éléments sur le contexte, c'est difficile de commenter.

Moui ... les erreurs sont placées dans un objet Properties lui-même à l'intérieur d'un objet "message" à l'évidence pour une exploitation ultérieure (affichage d'erreur, log, ...)

qu'est-ce qui te "tire l'oeil" (belle expression !) là dedans ?

Ah quand on passe après les copains, la principale difficulté consiste à comprendre ce qui se passait dans leur petit ciboulot quand ils ont codé... la deuxième difficulté c'est de se retenir de juger trop vite (ce serait pêcher d'orgueil ;) )

Je rejoint nepo.. on ne voit là qu'un catch (on ne sait même pas à quel try et à quel contexte il appartient...). C'est ce qu'il y a avant qui te dira si c'est bon ou pas.

Imagine qu'il ait une petite classe qui s'occupe de tous les accès fichiers, qui va donc forcement lever des exceptions (notfound, problème de droits d'accès, etc...). Il peut très bien soit les traiter et les injecter dans une "MyCorpException" pour alimenter un fichier d'erreur, soit les injecter directement dans une "MyCorpException" et les remonter en vue de les traiter dans son contrôleur par exemple. Si c'est le cas c'est pas une façon de faire qui me parait môche : c'est peut être pas la mieux mais ça sera toujours infiniment mieux que de ne rien faire :aie:

Essaye donc de retrouver à quel endroit il déclare ces fameuses "MyCorpException"?

pour plus de précisions "message" est une grosse enveloppe contenant des messages applicatifs, code d'erreur etc... (esb en sous main... pour l'instant). Moi ce qui me choqué été le fait de faire le catch sur 'exception', de traiter à l'intérieur une "sous exception" avec une analyse de chaine, puis faire un cast dessus, etc...
Je suis adepte de la dernière solution de "Enerian".

Aaahhh les joies de la maintenance ::lol:

Je travaille sur un projet qui à été créée il y à plus de 25 ans, il à connu plusieurs gros changements et une flopée de développeurs. Je peux te garantir que des fois tu tombes sur des perles :aie::aie:

Mais bon on est obligé de faire avec.. et même si parfois tu as envies de tout casser pour tout refaire, le problème est qu'il y à toujours des effets de bord imprévisible.. (et ton chef qui te cries dessus parce que tu as tout cassé :mouarf: )

Donc oui c'est moche, mais à mon avis le plus simple malheureusement reste de faire avec..

Mais bien sûr ce code peut être retouché en vu d'être amélioré.. de toute façon deux cerveaux valent toujours mieux qu'un! Cependant attention, ce que tu as entre les mains, c'est l'ouvrage d'un autre, je suppose qu'il n'y a pas seulement 3 lignes de code dans ce projet que l'on t'a demandé de reprendre.
De ce fait soit on te le confie car il y a un problème, et tu vas devoir débuguer dans la pire des conditions (tu devras répondre à la fameuse question "mais POURQUOI il a fait ça comme ça, le bougre!!" :aie: ), soit on te le confie pour continuer, ajouter, améliorer ... franchement un petit conseil, touche pas trop à ce qui marche ;)

Je considère toujours le travail des autres comme une structure en allumettes : touche à celle qui faut pas et tout s'effondre. Et à part exception divine, le code purement impeccable sur le plan de ce qui "est propre" ou ne l'est pas...ça n'existe pas :oops:

Si c'était ton code, alors là, oui, chercher à être le plus propre et le plus cohérent possible c'est la bonne attitude.

Merci pour vos retours.
J'amais bien une affichette qu'une collègue avait collé à son bureau qui disait en substance: "écrivez toujours votre code comme si un dangereux psychopathe va le lire" (je n'ai plus la phrase exacte). Allez affaire résolu, on glissera des modifs d'améliorations au fur et à mesure :) !

Hihi, un dangereux psychopathe , je l'aime bien celle là ;)

Bah, c'est mon secteur d'activité, hein. Vous savez jamais avec qui je vais travailler demain, ça peut être vous.