Coder proprement : évitez le bruit !

La publication de cet article m'a été motivée par l'article de François Wauquier Refactoring: Supprimmer le bruit qui a tout de suite fait résonner chez moi des concepts que je voulais partager ici depuis longtemps, et dont en voici quelques uns, en rebondissant sur certains thèmes abordés par François.

D'ailleurs, François mentionne essentiellement le code de test, mais pour moi, les règles sont les mêmes que le code soit de test ou de production !

Javadoc et commentaires

Pour moi, la meilleure JavaDoc est le code lui-même. Idem pour les commentaires. En plus, tout cela ne sera que rarement maintenu avec l'évolution du code, mieux vaut de loin un code explicite, avec des nommages (variables et méthodes) clairs, précis et parlants, quitte à être un peu long ... Un roman avec des phrases ...

J'aime bien le principe de Robert C. Martin dans son livre Coder Proprement (que je recommande !) pour qui le recours au commentaire est le signe d'un échec pour trouver de bons nommages.

Pour la JavaDoc, je différencie néanmoins le cas d'une librairie publique, ou du moins utilisées par d'autres équipes (par exemple), exposant une véritable API, où la JavaDoc peut avoir de la valeur ajoutée, mais à condition d'être complète, à jour, compréhensible, etc ... un vrai boulot !

Getter et setter

Je suis arrivé à la pratique suivante : si mon objet est un "objet de données" ne servant qu'au sein de mon package, aucune hésitation, pas de getter/setter et les attributs en portée "package" (donc sans préciser la portée). Par contre, s'il s'agit d'une API, là j'hésite un peu plus, surtout si mes objets doivent être immutables, dans ce cas, attributs privés fournis dans le construteur et accessible uniquement par les getter : 

public class User {

 private final String name;

 private final String phone;

 public User(String name, String phone) {
  this.name = name;
  this.phone = phone;
 }

 public String getName() {
  return name;
 }

 public String getPhone() {
  return phone;
 }
}

A noter tout de même que les getter/setter peuvent faciliter le débogage, ainsi que le refactoring (i.e. un getter qui n'en est finalement pas un ...). Donc, à voir selon les situations ...

Découpage des tests

Pour moi, la règle simple est la suivante : bien respecter le découpage d'un test en 3 phases, présentées sous forme de blocs séparés par une ligne vide :

1. préparation
2. test (et un seul)
3. vérifications (une ou plusieurs assertions)

Si nécessaire, une phase additionnelle de nettoyage peut être envisagée. Ne pas oublier que chaque test doit être indépendant, les tests doivent pouvoir être déroulé seul ou dans n'importe quel ordre.

J'ajouterai que si la phase de préparation est un peu longue, ne pas hésiter à la reporter (tout ou partie) dans des méthodes pour augmenter sa simplicité et donc sa lisibilité.

Constructeur vide et "super()"

Je ne les mets jamais, mais effectivement, il faut bien se poser la question lorsqu'on ajoute un constructeur avec des paramètres : ne faut-il pas un constructeur vide également (piège) ?

De même, je supprime les "super()" en début de constructeur, totalement inutile et alimentant le "bruit" !

Messages d'assertions

Tiens, c'est marrant, jusqu'à très récemment, dans mes formations ou interventions, je préconisais l'ajout systématique d'un message dans les assertions, pour bien expliquer la situation. Mais lors de mes 2 dernières interventions, j'ai remis cela en question.

En effet, je me suis rendu compte de la difficulté pour les développeurs pour comprendre le rôle de ce message (quand est-il affiché et à quoi sert-il ?) et trouver le bon texte (risque de message inversé ou innaproprié). Et lorsque le principe est compris, les messages peuvent vite se retrouver très longs, nuisant ainsi complètement à la lisibilité du test. Sans compter, les duplications de tests où le message n'est pas mis à jour.

J'en arrive finalement à changer d'avis et opter pour l'absence de messages, mais avec des nommages clairs et explicites (ici encore !), éventuellement par l'usage de méthodes personnalisées permettant de regrouper plusieurs assertions :

    assertThatObjectContainsTheThreeFirstNumbers(generatedObject);

Variable locales et nommages

J'adhère à l'approche de Robert C. Martin qui démontre que l'utilisation des variables locales est dangereuse et qu'il faut donc limiter leur usage. Donc, on les supprime, sauf si la ligne générée devient vraiment trop longue ou si la présence d'une variable locale intermédiaire peut aider à la lisibilité et la compréhension du code.

Dans ce cas, le nommage est très important, éviter les noms courts, voir très court à une ou quelques lettres. Et comme le dit François Wauquier, "Le meilleur nommage pour une variable est le nom de sa classe". En plus, Eclipse le fait automatiquement avec la commande "Refactor / Extract Local Variable" (ALT + SHIFT + L).

Conclusions

Le refactoring est indispensable et doit être quotidien dans le travail d'un développeur. Il est incontournable pour faire évoluer un code existant, notamment pour lui ajouter de nouvelles fonctionnalités. Le risque varie selon le type de refactoring effectué (renommages, extraction en méthodes, reprise d'algorithme, ...) et les modifications évoquées par François Wauquier et reprises en partie ici sont à faible risque.

J'aime beaucoup l'analogie trouvé par François avec le bruit, j'aurais aimé la trouver moi-même, je reprendrais l'idée, bravo et merci !

Et comme François propose de Supprimer le bruit, je pousse un peu plus loin en proposant, en plus, sur du nouveau code, d' Eviter le bruit ! ;-)