«

»

Jul
09

Refactoring: Exceptions ou conditions imbriquées?

Dans un de nos projets, j’ai été amené à travailler sur du code faisant différentes vérifications quand l’utilisateur veut changer son mot de passe. Après avoir ajouté une vérification supplémentaire et les tests automatisés qui vont bien. Je me suis retrouvé avec quelque chose de pas très propre qu’il allait falloir refactorer. (La vérification n’est pas terrible car elle considère qu’un échec d’insertion de la part de l’ORM est forcément dû à un mot de passe trop court, à revoir mais c’est n’est pas le sujet de ce post)

Après une première étape de refactoring j’ai extrait autant que je pouvais de méthodes et de conditions pour donner plus de sémantique au code, mais je trouvais que les conditions imbriquées rendaient difficile la compréhension.

Je me suis souvenu avoir lu dans Clean Code (écrit par Uncle Bob) que les exceptions étaient un moyen de nettoyer du code faisant de multiples vérifications.  Voilà donc une occasion d’essayer.

L’état actuel, après utilisation des exceptions pour supprimer les if imbriqués.

La question est quelle version trouvez-vous la plus lisible? Celle avec les conditions imbriquées ou les exceptions? Que ce sois pour l’une ou l’autre, n’hésitez par à suggérer des améliorations sur des simplifications que j’ai dû oublier. Si vous avez une approche totalement différente, ça serai également très intéressant!

Victor

6 comments

  1. Piarulli Pierre-Alexandre says:

    https://gist.github.com/tuxayo/9e76ec71206d3063e491

    je ne saurais dire exactement pourquoi
    mais la ligne 10 et 45 me perturbe.

    j’ai envie d’extract ligne 6 à 9

    l37 variable temporaire pour une convetion rails..
    je vois bien des unless à la place des if not

    il y a 3 methode qui ce ressemble….
    hum hum haaaaa
    il y a un truc je le sent.
    http://akodieon.deviantart.com/art/Thinking-Deep-Thoughts-305501185

    bon travail de comparaison en tout cas

    1. Benoit Gantaume says:

      Salut,
      Effectivement la variable de la ligne 37 pourrait être “inlinée” dans la condition.
      L’usage de “unless” à la place de “if not” me semble également plus :ruby.
      Merci pour ton retour.
      #++

      1. tuxayo says:

        Il faut que je me force à mettre des unless dès que possible, je n’ai pas encore assez l’habitude pour les lire naturellement?

        1. Benoit Gantaume says:

          Ce n’est pas tant une histoire de se forcer que d’en prendre l’habitude…

    2. tuxayo says:

      “j’ai envie d’extract ligne 6 à 9″
      Les extraire car ce sont ces lignes qui peuvent lever une exception?

      “l37 variable temporaire pour une convetion rails..”
      Merci, j’avais peur que ça ne soit pas assez clair mais si c’est déjà une convention pas besoin de variable en effet.

  2. Benoit Gantaume says:

    Salut,
    Merci pour cet exemple de refactorisation. Pour ma part j’avais une préférence au début pour la première étape. Suivre de flot d’exécution me semblait plus simple.
    Mais plus j’y pense et plus je trouve la seconde agréable et élégante. Finalement la seule chose qui me questionne est l’usage d’exceptions pour gérer des cas normaux d’erreur.
    J’aurais eu tendance à garder les exceptions pour les cas… Exceptionnels…
    Qu’en pensez-vous?
    Comment utilisez-vous les exceptions?
    #++

Leave a Reply

Your email address will not be published.

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>