Clean Code avec le Kata Video Store » Refacto d’oncle Bob (suite)

Organisation des fonctions

Dans la décomposition précédente, remarquez l’ordonnancement des fonctions (Function Structure) : il suit la Step-down Rule. Les fonctions sont triées en suivant l’ordre de leur appel, de manière récursive c’est-à-dire par niveau d’abstraction descendant. Plus on descend dans le fichier, plus on entre dans le détail d’implémentation des fonctions appelées plus haut. Cela facilite la lecture qui se fait comme celle d’un article de journal : les généralités d’abord puis le détail. La proximité créant du lien, je trouve cette organisation plus cohérente que le classement par ordre alphabétique auquel j’adhérais auparavant. Il faut juste faire attention à bien paramétrer ses outils de formatage du code pour que cela soit compatible avec la step-down rule, comme constaté par l’auteur de cette question sur Stack Overflow.

Un autre point en rapport avec l’organisation des fonctions : la répartition des fonctions entre différentes classes. On cherche de la cohérence au sein d’une classe, par rapport à ses données et entre ses fonctionnalités. Il est préférable de placer une fonction au plus près des données qu’elle utilise, permettant de les encapsuler dans la classe. Oncle Bob tombe successivement sur deux occasions d’améliorer le design en ce sens.

rental.getMovie().getTitle()➚ est une violation de la Loi de Demeter que l’on résume en : « ne pas connaître les voisins des voisins ». Chaque classe ne communique qu’avec les classes qu’elle connaît. C’est un principe de connaissance minimale et cela permet de réduire le couplage. Pour corriger le code précédent, on cherche à avoir directement rental.getTitle()➚. Il suffit donc créer la méthode getTitle() { return movie.getTitle(); }➚. C’est du simple Forwarding, souvent appelé à tort Délégation.

Lorsqu’une méthode/fonction utilise plus les données d’une autre classe/module que de celle/celui la contenant, cela sent la Feature Envy. C’est le cas de determineAmount(rental) et determineFrequentRenterPoints(rental)➚ qui utilisent les champs de l’objet rental plutôt que de leur classe Statement. Oncle Bob utilise alors le refacto Move Instance Method en spécifiant Rental comme type de destination, celui-ci étant proposé par l’IDE par analyse des types utilisés dans la méthode. Cela veut dire que le refacto marche aussi pour des champs de la classe.

Refacto du switch

Oncle Bob applique le refacto Replace Conditional with Polymorphism pour remplacer le switch dans determineAmount() et determineFrequentRenterPoints() par un polymorphic dispatch i.e. par l’appel à movie.determineAmount() où la méthode est abstraite et définie dans le type Movie mais implémentée dans des classes héritant de Movie. Cela permet de suivre le DIP , (le D de SOLID).

👉 Note
Le polymorphic dispatch est un dynamic dispatch : ce n’est qu’au runtime qu’est connu le type concret de l’objet et donc l’implémentation de la méthode virtuelle, localisée dans l’une des classes de la hiérarchie définie par héritage. Les langages orientés-objet nous offrent donc une certaine souplesse à même de satisfaire l’OCP(le O de SOLID), le tout avec sécurité : on est assuré que la méthode existe.

⚠️ Attention
Ce n’est pas la même chose dans ces deux cas-ci :
– En C#, les méthodes d’extension permettent d’ajouter des méthodes à un type, enum et interface inclus. C’est pratique de pouvoir écrire o.Method() plutôt que Method(o), surtout pour enchaîner les appels comme avec les requêtes LINQ : items.Where(...).Select(...).First(). Cependant, ce n’est que du sucre syntaxique. Les méthodes d’extension ne sont pas virtuelles et donc ne peuvent pas participer à un polymorphic dispatch. Pour le retrouver, on s’orientera plutôt vers le pattern décorateur.
– Les langages dynamiques, y compris C# avec le type dynamic, permettent d’appeler un membre d’un objet non connu à la compilation. Cela autorise encore plus de souplesse mais au détriment de la sécurité offerte par le compilateur. En C#, en alternative au pattern Visiteur et son « double dispatch », on peut utiliser une conversion en dynamic</codeem>(ligne 12)</em> pour éviter une série de <em>down casts</emem>(lignes 4, 5)</em> : </p></blockquote> <pre><code class="language-cs" highlight="4,5,12"> // Down casts même si avec pattern matching (syntaxe plus élégante qu’avant) void Print(Shape shape) { switch (shape) { case Square s: Print(s); break; case Circle c: Print(c); break; default: // Gestion explicite des autres types } } // Dynamic dispatch (unsafe pour les types non gérés) void PrintDynamic(Shape shape) { Print((dynamic) shape); } void Print(Square square) {...} void Print(Circle circle) {...}

Pour effectuer le refacto Replace Conditional with Polymorphism, oncle Bob crée autant de classes héritant de Movie que de cas dans le switch : NewReleaseMovie, ChildrensMovie, RegularMovie. Pour cela, de nouveau il bascule côté tests pour piloter le design du code de production. L’astuce consiste à changer l’initialisation des champs de type Movie : au lieu d’appeler le constructeur de la classe de base, Movie, on appelle le constructeur de la future classe. Cela permet de la créer en s’appuyant sur l’IDE pour automatiser cette tâche.

Exemple pour le champ childrens :
– Avant : childrens = new Movie("Childrens", Movie.CHILDRENS)
– Après : childrens = new ChildrensMovie("Childrens")

ChildrensMovie n’existe pas encore. L’IDE le colore en rouge et propose un quick fix pour créer la classe ChildrensMovie qui dérivera déjà de Movie du fait du type du champ childrens. Il suffit alors de compléter le constructeur pour appeler celui de base en spécifiant le title et le priceCode désormais implicite :

    public class ChildrensMovie extends Movie {
      public ChildrensMovie(String title) {
        super(title, Movie.CHILDRENS);
      }
    }

💡 Astuce
De cette manière, le temps passé avec les tests en rouge est minime. Très vite, on revient en vert et on récupère notre système opérationnel. Évidemment, cela suppose de jouer les tests très régulièrement, une bonne pratique à acquérir.

Oncle Bob effectue alors un Push Members Down des méthodes determineXxx() en sélectionnant l’option Keep abstract pour conserver la méthode abstraite dans Movie. Dès lors, chaque classe dérivée de Movie récupère une copie complète de ces méthodes, y compris les switch/if. Ne reste plus qu’à les épurer des cas qui ne les concernent pas. Pour cela, oncle Bob active la couverture des tests pour visualiser le code non couvert qui peut être supprimé après vérification (Souvenez-vous du cas non couvert par les tests). Enfin on peut supprimer le champ priceCode➚ et les constantes correspondantes telles que CHILDRENS, les types dérivées de Movie portant intrinsèquement cette information.

Le refacto du Replace Conditional with Polymorphism nécessite donc plusieurs opérations à la suite. Il faut orchestrer différents refactos gérés par l’IDE, guidés par de subtiles modifications manuelles du code. Je trouve la façon de faire d’oncle Bob assez efficace. Il y a sûrement d’autres procédés possibles, peut-être plus efficaces, d’autant que les refactos gérés par nos IDEs sont de plus en plus perfectionnés.

☝️ Note
On retrouve le pattern Template Method dans sa version classique qui s’appuie sur l’héritage : les méthodes determine*() de Movie sont abstraites et implémentées dans les classes dérivées. On n’a qu’un seul niveau de profondeur et les classes dérivées ne font rien d’autre qu’implémenter les méthodes abstraites de leur classe mère. C’est donc une utilisation sécurisée de l’héritage. Néanmoins, qui dit héritage, dit problèmes en germination. Pour éviter cela, deux choix s’offrent à nous :
– La classe Movie ne contient que le champ title. Cela représente la mutualisation de vraiment peu de code. Il est donc envisageable de transformer Movie en pure interface et de laisser les classes l’implémentant initialiser le title. La duplication de code, très faible, est acceptable.
– On peut s’orienter vers un autre pattern comportemental s’appuyant lui sur la composition, la Strategy. On aurait alors une ou deux classes permettant de déterminer le montant et les points de fidélité selon différentes stratégies : films pour enfants, nouvelles sorties ou standard. Autre avantage : pouvoir mutualiser les instances de stratégie(s) entre les films.

Résultat

Design

Oncle Bob a adressé bon nombre des problèmes que l’on avait mentionnés. Le code et le design obtenus respectent les principes clean code, le tout en 1h !

• Toutes les fonctions sont courtes, simples, expressives : leur nom exprime ce qu’elles font (le What), pas comment elles le font (le How).
• Le comportement est mieux réparti entre les classes.

Il reste des points améliorables que l’on a évoqués précédemment : couplage temporel, encapsulation ébréchée, usage de l’héritage. Nous les traiterons dans un prochain article.

L’art d’utiliser son IDE

Oncle Bob nous montre une succession de techniques de refactoring effectuées dans IntelliJ. C’est du grand art dont il faut s’inspirer, y compris en l’observant faire encore quelques opérations manuelles, automatisables dans l’IDE 😜.

☝️ Côté .NET
On a le choix : ReSharper ou CodeRush intégré à Visual Studio, le petit nouveau Rider, IDE qui offre une UX différente de Visual Studio, plus de stabilité (qui ne connaît pas les gels de VS qui désespérément ne répond plus !?), mais qui gagnerait à pouvoir lancer des extensions de Visual Studio. Visual Studio et VSCode ne sont pas en reste en proposant régulièrement des fonctionnalités déjà offertes par les concurrents.

Malgré quelques imperfections, les IDEs modernes nous offrent un gain de productivité et de sécurité, pour peu que l’on acquière la discipline de les utiliser et de manière efficace : en utilisant les raccourcis clavier. On n’est pas obligé de connaître tous les refactos possibles. Pour commencer, on peut se limiter à ces 6 refactos essentiels : Rename, Inline, Extract Method, Introduce Variable, Introduce Parameter, Introduce Field. L’important est de s’appuyer sur un ou plusieurs IDEs et de maîtriser les fonctionnalités essentielles pour le refacto sécurisé et une productivité boostée.

Cela n’empêche pas de chercher à découvrir d’autres fonctionnalités. À ce titre, je vous en partage deux que je pense peu connus et que j’utilise avec plaisir voire jubilation en pensant aux fastidieuses tâches répétitives que cela m’épargne :

• Vous connaissez probablement l’édition en colonne. Son prolongement naturel est l’édition multi-curseurs. Elle se fait soit au moyen de clics à la souris, imprécis (mais on n’a parfois pas le choix, cf. GIF animé ci-dessous), en se basant sur la sélection courante et en ajoutant les occurrences à la sélection qui se segmente en plusieurs bouts de code éditables conjointement, indépendamment de leur positionnement en colonne. Cette fonctionnalité est disponible entre autres dans les produits de JetBrains (IntelliJ, Rider…), Visual Studio Code et Visual Studio 2017 depuis l’update 15.8, enfin ! Le seul point négatif : la terminologie et les raccourcis clavier sont différents selon chaque éditeur de texte.

• La complétion de code est une fonctionnalité classique. On la connaît avec « l’IntelliSense » et les snippets. Une facette moins connue est la « postfix completion ». Son action est rétroactive, en s’appliquant sur le bloc de code précédant le curseur pour le transformer : comme condition d’un if, en tant que variable extraite, booléen inversé… La postfix completion s’effectue en tapant un . suivi du code du template comme pour un snippet, respectivement .if, .var, .not pour les trois exemples précédents. Son objectif est d’éviter des retours en arrière du curseur. JetBrains propose la postfix completion depuis 2014 dans ses produits phares : IntelliJ, ReSharper, Rider. Je ne connais pas d’autre produit qui le fasse. C’est donc une manière de compléter du code. À noter des petits imprévus en l’utilisant, le bloc de code attendu n’étant finalement pas celui pris en compte.

Conclusion

L’étude du kata VideoStore et sa réalisation par oncle Bob a tenu ses promesses en se montrant riche d’enseignements non seulement sur les code smells et le refacto de code legacy, le bon usage des IDEs, mais aussi sur le clean code, les tests unitaires, le TDD, la programmation orientée-objet, les principes SOLID et les design patterns.

On a également évoqué la problématique de savoir quand arrêter le refacto. Un équilibre est à trouver entre le temps dont on dispose, les exigences et objectifs de qualité définis sur la base de code (codebase) par soi-même ou encore mieux partagés par l’équipe de développement. Oncle Bob s’est donné une heure et réalise une belle performance : si je devais faire la revue de code du pull request (PR) correspondant, je pourrais au plus émettre quelques suggestions et valider la PR.

Quand un développeur (soi-même, un autre développeur, voire une personne non technique) passera sur ce code, il devrait être plus à même de comprendre cette version du code plutôt que la précédente. On applique la règle du boy-scout de manière intelligente, en se projetant dans le futur, en comprenant l’appartenance collective du code.

En même temps, après avoir évoqué ces améliorations possibles, en restant dans le cadre d’un exercice porteur d’enseignements, je vous propose de continuer le refacto, en C# d’un côté mais également en TypeScript. Ce sera à mon tour de ne pas aller trop loin. La suite dans un prochain article.
En attendant, j’espère que vous avez apprécié ces articles. Si cela induit en vous de nouvelles pratiques, un état d’esprit de craftsmanship, mon objectif de diffuser ce que l’on a vu au sein des masterclasses clean code sera atteint 😄

© SOAT
Toute reproduction est interdite sans autorisation de l’auteur.