Je voulais vous présenter un type de réfactoring que j'ai experimenté récemment: Supprimmer le bruit.
Avant tout je précise ce que j'entend par bruit. C'est un terme qui provient du traitement du signal, et qui signifie parasite, la partie du signal qui n'apporte pas d'information. http://fr.wikipedia.org/wiki/Rapport_signal_sur_bruit
Pour du code, le bruit est l'ensemble des caractères qui n'apportent pas d'information, et ralentissent le compréhension du code.
Je l'ai utilisé sur des tests d'integration en regression dont je n'arrivais pas à comprendre le sens. J'ai donc appliqué des améliorations sur les classes de test. Les modifications présentées ci dessous ont la caractéristique de ne pas changer le comportement de production, et permettent donc d'avancer en confiance.
Visibilité
Supprimmer la visibilité dans les interfaces et les attributs des classes de test.
Javadoc vide
Déjà, la javadoc sur une application sans API publique, cela n'apporte que rarement quelque chose de plus q'un bon nommage. En plus, quand c'est sur un méthode de test, et que en plus elle est vide, alors là, je supprime sans etat d'ame.
Getter inutile
Certains tests injectent ou se font injecter les attributs à partir d'un contexte spring ou autre. Le getter n'est pas nécessaire pour cela.
Assertions verbeuses
Pour vérifier la non-nullité et la taille d'un collection, on peut fairre l'assertion en junit
assertNotNull(list);
assertEquals(2, list.size());
... ou avec Fest Assert
assertThat(list).hasSize(2);
Découpage de test
Parfois vous vous rendez compte qu'une methode de test appelle plusieurs fois la même méthode avec des paramêtres différents. Cela signifie que ce sont plusieurs tests distincts. Il convient donc de découper le test en plusieurs méthodes avec un nommage décrivant ces différents cas de test.
Formattage BDD
Cela facilite également la lecture de passer une ligne entre les différents blocs du test:
- initialisation (given)
- action (when)
- assertions (then)
J'ai parfois utilisé des commentaires pour séparer ces blocs, mais lorsque cette simple règle de formattage est connu, cela est beaucoup plus efficace. Par ailleurs, certains framework de mock comme Mockito proposent des méthode utilisant le nommage BDD
Suppression des catch-fail
J'ai souvent retrouvé cet anti pattern du catch-fail, voire du catch-log-fail. Je ne vois pas l'interêt d'avoir un failure plutôt q'une erreur lorsque le méthode testée renverra une erreur. Quelle que soit la manière dont le test sera lancée, cette erreur sera remontée. Par contre je suis sûr que cela freine la lecture du code. Supprimé!
Regroupement des exceptions
Dans la close throws d'une méthode de test, peu m'importe de lister les exceptions que peut renvoyer cette méthode, un 'throws Exception' suffit.
Utilisation de expected pour les exceptions attendues
Transformer
@Test
public void testCasLimite() throws Exception {
try {
service.doSomething();
fail();
} catch (ExpectedException e) {
// success
}
}
... en
@Test(expected=ExpectedException.class)
public void testCasLimite() throws Exception {
service.doSomething();
}
Suppression du constructeur vide
Un constructeur d'une classe de test, vide ou avec le paramêtre 'name', mais pas de code, je le supprimme aussi. (Je conserve par contre volontier un constructeur vide dans une classe de production, afin de faciliter la navigation dans le code, et ne pas oublier ce constructeur vide au cas où l'on en ajoute un autre, mais que l'objet en question est serialisé, et a besoin d'un constructeur vide)
Suppression du message d'assertion
Le message d'assertion apporte peu d'informations par rapport à l'assertion elle-même, et au message construit par cette assertion lorsqu'elle echoue (ex: expected: 1 but was: 2). Parfois le code evolue, mais le message est resté identique, si bien qu'il nomme les objets avec un certain nom au moment où l'on écrit le test, mais que ce message apoprte aujourd'hui une confusion. Un message peut néanmoins être utile s'il apporte des valeurs importantes qui ne sont pas dans l'assertion elle-même.
Renommage de variable locale
Le meilleur nommage pour une variable est le nom de sa classe.
Utiliser les imports statics
Transformer
Assert.assertEquals("expected",actual);
... en
assertEquals("expected",actual);
Les plus attentifs et courageux qui seront arrivé jusqu'ici ont été indigné par cette approche. Quoi? du refactoring sur une barre rouge? n'importe quoi ! Tel que l'a rappellé récement Martin Fowler dans cet article sur le refactoring opportuniste, le refactoring s'effectue sur une barre verte. J' ai fait une entorse à cette règle en choisissant les améliorations qui me semblait sans danger.
J'espère que cela vous donnera des idées pour rendre votre code plus propre. Les commentaires sont les bienvenus.
Pour aller plus loin
Aucun commentaire:
Enregistrer un commentaire
Feedback...