C’est en discutant avec un collègue de refactoring et de qualité de code que m’est venue l’idée pour ce billet. Dans ce billet, je présente quelques transformations que j’ai l’habitude de faire à du code existant. Il s’agit aussi de la façon que j’ai d’écrire mon C#.
Pour moi, le critère le plus important en revue de code est de comprendre facilement l’intention de l’auteur vis-à-vis le problème à résoudre. Au moment de faire une revue de code, ma règle d’or est que je dois être en mesure d’avoir une bonne idée du problème à résoudre uniquement à la lecture de vos commits. Imaginez, si je ne suis pas en mesure de le deviner à cet instant, la tête que fera le développeur dans six mois lorsqu’il sera temps de faire évoluer vos changements.
Le code que vous produisez est la meilleure documentation que vous pouvez laisser aux développeurs qui vont vous succéder dans le futur. Assurez-vous qu’elle soit la plus claire possible. Évidemment, tout est une question de contexte. Autrement, un hack est une mesure exceptionnelle pour un cas exceptionnel. Il faut toujours garder cela en tête.
Alors donc, sans plus tarder, voici ce dont il est question!
Ordonner les directives using
L’ordonnancement des directives using a un seul but: donner un peu l’impression d’ordre et de rangement dans le code. En plus, ce refactoring est possible dans Visual Studio sans l’intervention de ReSharper. Il suffit de faire un clic droit sur les usings et de visiter le menu Organize Usings.
Sceptique? Ne me dites pas que vous préférez
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
using System.Linq; | |
using Castle.MicroKernel.Resolvers.SpecializedResolvers; | |
using System.Net.Http.Formatting; | |
using System.Reflection; | |
using Castle.MicroKernel.Registration; | |
using Castle.Windsor; | |
using Castle.Windsor.Installer; | |
using Core; | |
using Filters; | |
using Formatters; | |
using System.Web.Http; | |
using System.Web.Http.Dispatcher; | |
using Plumbing; |
à celle-ci
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
using Castle.MicroKernel.Registration; | |
using Castle.MicroKernel.Resolvers.SpecializedResolvers; | |
using Castle.Windsor; | |
using Castle.Windsor.Installer; | |
using Core; | |
using Filters; | |
using Formatters; | |
using Plumbing; | |
using System.Linq; | |
using System.Net.Http.Formatting; | |
using System.Reflection; | |
using System.Web.Http; | |
using System.Web.Http.Dispatcher; |
n’est-ce pas?
Toujours inclure les accolades
Selon moi, il n’y a pas de bonnes raisons de ne pas vouloir mettre d’accolades pour entourer les différents mots clés d’instructions de C# (ou de tout autre langage équivalent).
Le cas le plus commun est l’écriture d’if simple où la condition sert à protéger l’exécution d’une seule ligne de code comme ceci.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
if (batch != null && batch.Any()) | |
return batch; |
En général, ça marche bien. Là où c’est problématique, c’est à l’instant où vous allez voir ajouter une deuxième instruction sous cet if. Du point de vue de la maintenabilité, c’est un non-sens. Incluez toujours les accolades, vous allez vous sauver des maux de têtes à long terme.
Des propriétés ou instantiantions avec des accolades et des sauts de lignes
Du code, c’est comme une plante, ça a besoin de respirer. Vous avez probablement vu, vous aussi, un bon lot de propriétés toutes petites, entassées dans un amalgame d’accolades sur une seule ligne. Je me dis toujours que ce n’est pas 4-5 lignes de code avec des accolades qui va faire une différence. En plus, on gagne en visibilité et en clarté.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
protected string SectionName | |
{ | |
get | |
{ | |
return SectionReferential.HomePage; | |
} | |
} |
Sauts de ligne avec l’opérateur ?:
L’introduction de l’opérateur ?: a permis aux développeurs C# ce simplifier l’écriture de certaines conditions en donnant un raccourci syntaxique. Il est très pratique à utiliser. En particulier lorsque vous avez une seule condition à tester.
Dans ce cas-ci, aussi, je préfère avoir les branches d’exécution sur des lignes distinctes. Cela a pour avantage de distinguer clairement les trois segments en relation avec cet opérateur.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
string.IsNullOrWhiteSpace(environment) | |
? environment.ToLowerInvariant() | |
: CurrentEnvironment; |
Chaînage des méthodes
J’ai eu énormément de discussion, dans le passé, sur cette façon de formater les appels de méthodes chaînées. Je trouve que d’avoir des appels comme le suivant sur une seule ligne révèle très peu sur les intentions du code. Comme si l’auteur tentait de cacher quelque chose dans un tas d’instructions qui se suivent.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Repository.GetAllById(24).Where(p => p.Country == "Canada").Select(p => p.UniqueIdentifier).ToList() |
Je préfère avoir mes méthodes chaînées mises en page de cette façon. De plus, l’avantage n’est pas seulement pour la lisibilité. Lorsqu’il est temps de modifier un des appels du groupe, il est plus facile de procéder à modifier une seule ligne que tout le groupe sur la même ligne.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Repository | |
.GetAllById(24) | |
.Where(p => p.Country == "Canada") | |
.Select(p => p.UniqueIdentifier) | |
.ToList() |
Toujours éliminer les lignes vides superflues
À l’œil, cela ne trompe pas. Lorsqu’une classe est correctement espacée, ça donne toujours une bonne impression au moment de faire une revue de code. C’est comme entrer dans une maison qui sent bon.
C’est facile de laisser glisser un espace ou deux par inadvertance. Lorsque je repasse les sauts de ligne dans une classe, j’essaie de m’en tenir à la règle de pouce suivante:
- un saut de ligne entre les méthodes
- un saut de ligne entre les usings et la déclaration de classe ou de namespace
- éliminer le saut de ligne superflu en fin de classe
Je suis du même avis que toi pour la présentation du code au niveau des accolades pour un if simple, le chaînage LINQ et pour le if inline!
Bonne réflexion!
Selon moi, il faudrait que les langages se débarassent completement de cette possibilité d’ecrire un IF sans accolades. C’est une abbération qui fait gagner 1 seconde au moment de l’ecriture mais qui peut engendrer des bugs difficiles à trouver sur le long terme.