alecharp.fr

Writing code crap and refactoring painlessly

Quand on écrit du code, il y a deux choses de mon point de vue qui peuvent nous conduire à l’auto-censure :

  • ne pas savoir écrire du code,
  • penser qu’on ne sait pas écrire du code propre.

Dans un cas comme dans l’autre, le résultat, si c’est le cas, est qu’il vaut mieux s’abstenir, sinon ça donne ça codecrap.com. Sauf que si on écrit jamais on ne saura jamais écrire, dixit le veille adage “c’est en forgeant qu’on devient forgeron.”

Donc je pense qu’il y a au moins une solution pour chaque cause.

Solutions

Je ne sais pas écrire du code

Lisez, allez aux conférences, retournez à l’école ou en formation. Il n’y a pas de mal à ça. Bien au contraire. On voit d’ailleurs plein de très bon développeurs Java suivre une formation (e-learning ou autre) pour mettre les mains dans le cambouis de Scala.

Je pense que je ne sais pas écrire du code propre

De même, pas de miracle, lisez, allez aux conférences, etc. Autres choses que vous pouvez faire, c’est vous faire relire : en travaillant à plusieurs, vous aurez plusieurs façon de voir les choses et donc vous serez amené à les essayer et voir quelle est la meilleure.

Si, par contre, vous être comme moi et que vous avez des collègues avec le “code” sur la main, alors écoutez-les, proposez-leur de participer à un projet perso pour avoir un retour, etc.

Extra

Dans tous les cas, certains outils et bonnes pratiques pourront vous aidez sur le long chemin de l’apprentissage : Git, un IDE maitrisé, des tests, un système de build automatisé, un code-reviewer. Je pense que tout ça mérite un autre post, donc j’y reviendrai.

Mise en pratique

Imaginons que vous souhaitez mettre en place un système qui va vous permettre de discuter (grosse simplification de mon cas pratique), mais comme vous êtes gentils, vous pensez aux étrangers.

Le code final sera surement un peu over-designed par rapport au cas d’utilisation que j’ai décrit mais l’important dans un voyage ce n’est pas la destination mais le chemin pour y parvenir.

Étape 1 : simple, efficace

On commence le code : 

1
2
3
4
5

public class Main {
	public static void main(String[] argv) {
		System.out.println("Bonjour!");
	}
}

Étape 2 : tests

On va dire que l’on veut pouvoir tester notre message affiché simplement, donc on revoit notre copie pour avoir une classe NiceGuy pour nous aider :

1
2
3
4
5

public class NiceGuy {
	public String sayHi() {
		return "Bonjour!";
	}
}

On écrit donc notre test, ainsi nous aurons un état que nous pourrons valider et ainsi éviter de contrarier le bon fonctionnement lorsque nous ferons évoluer notre application :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10

public class NiceGuyTest {
	@Test
	public void sayHi() {
		assertThat("Bonjour!").isEqualTo(new NiceGuy().sayHi());
	}
	@Test
	public void notSayingHi() {
		assertThat("Au revoir!").isNotEqualTo(new NiceGuy().sayHi());
	}
}

* utilization de assertj-core et junit pour les tests unitaires.

1
2
3
4
5

public class Main {
	public static void main(String[] argv) {
		System.out.println(new NiceGuy().sayHi());
	}
}

Étape 3 : “parle-moi comme je parle”

Bon, on veut plusieurs langues possibles à partir d’un argument de la ligne de commande :

1
2
3
4
5

public class Main {
	public static void main(String[] argv) {
		System.out.println(new NiceGuy(argv[0]).sayHi());
	}
}

On va donc permettre d’instancier un NiceGuy en précisant un langage à utiliser.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13

public class NiceGuy {
	private final String locale;

	public NiceGuy() { this(null); }
	public NiceGuy(String locale) { this.locale = locale; }

	public String sayHi() {
		if (locale == null || "fr_FR".equals(locale) || "fr".equalsIgnoreCase(locale)) {
			return "Bonjour!";
		}
		throw new IllegalStateException("The locale is not recognized");
	}
}

Là, notre test nous dit que tout va bien. Cool, nous avons donc amélioré le code sans régression. Mais testons un peu plus :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21

public class NiceGuyTest {
	@Test
	public void sayHi() {
		assertThat("Bonjour!").isEqualTo(new NiceGuy().sayHi());
	}

	@Test
	public void sayHiFR() {
		assertThat("Bonjour!").isEqualTo(new NiceGuy("fr").sayHi());
	}

	@Test
	public void notSayingHi() {
		assertThat("Au revoir!").isNotEqualTo(new NiceGuy().sayHi());
	}

	@Test(expected = IllegalStateException.class)
	public void unknownLocale() {
		assertThat("Au revoir!").isEqualTo(new NiceGuy("etps").sayHi());
	}
}

Maintenant, nous sommes sûrs que lorsque nous précisons un langage ou non, nous avons bien un retour en français. En plus, si un langage n’est pas connu, nous avons bien l’erreur que nous attendions. Tout cela semble parfait.

Donc comme “we areu completely bilinguale” (complètement bilingue en français dans l’accent), on peut faire le code pour parler en Anglais :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17

public class NiceGuy {
	private final String locale;
	public NiceGuy() { this(null); }
	public NiceGuy(String locale) {
		this.locale = locale;
	}

	public String sayHi() {
		if (locale == null || "fr_FR".equals(locale) || "fr".equalsIgnoreCase(locale)) {
			return "Bonjour!";
		}
		if ("en_US".equals(locale) || "us".equalsIgnoreCase(locale)) {
			return "Hi motherfucker!";
		}
		throw new IllegalStateException("The locale is not recognized");
	}
}

On complète notre test pour être sûr que nous avons un code validé :

1
2
3
4
5
6
7
8

public class NiceGuyTest {
	…
	@Test
	public void sayHiUS() {
		assertThat("Au revoir!").isNotEqualTo(new NiceGuy("us").sayHi());
		assertThat("Hi motherfucker!").isEqualTo(new NiceGuy("us").sayHi());
	}
}

Bon ok maintenant ça fonctionne, mais on se rend bien compte que la méthode sayHi va vite devenir très longue et potentiellement compliquée à suivre dans le temps.

Étape 4 : et dans 6 mois, je ferai comment ?

Donc pensons 3 secondes. Il nous faudrait un système pour avoir différentes façons de répondre à sayHi et de découvrir ces propositions tout seul (ou presque).

Je reviendrai sur le presque. Intéressons-nous à la transformation de notre implémentation. On peut utiliser le mécanisme d’héritage ou d’implémentation. Compte tenu du fait qu’aucun langage n’aura de code en commun, je choisis de créer une interface à partir de NiceGuy. Pour des raisons de nommage, transformons l’actuelle NiceGuy en FrenchNiceGuy :

1
2
3
4
5
6

public class FrenchNiceGuy {

	public String sayHi() {
		return "Bonjour!";
	}
}

Puis on peut faire une “extraction” vers une interface pour créer l’interface NiceGuy :

1
2
3

public interface NiceGuy {
	String sayHi();
}

Si on revient vers FrenchNiceGuy on peut voir que la classe implémente notre nouvelle interface. Créons notre implémentation AmericanNotSoNiceGuy :

1
2
3
4
5
6
7

public class AmericanNotSoNiceGuy implements NiceGuy {

	@Override
	public String sayHi() {
		return "Hi motherfucker!";
	}
}

On doit transformer nos tests pour qu’ils valident nos implémentations :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17

public class NiceGuyTest {
	@Test
	public void sayHi() throws Exception {
		assertThat("Bonjour!").isEqualTo(new FrenchNiceGuy().sayHi());
	}

	@Test
	public void notSayingHi() throws Exception {
		assertThat("Au revoir!").isNotEqualTo(new FrenchNiceGuy().sayHi());
	}

	@Test
	public void sayHiUS() throws Exception {
		assertThat("Au revoir!").isNotEqualTo(new AmericanNotSoNiceGuy().sayHi());
		assertThat("Hi motherfucker!").isEqualTo(new AmericanNotSoNiceGuy().sayHi());
	}
}

On peut donc faire une liste d’implémentations. Il faut que cette liste fasse partie du langage de l’implémentation. Ceci est le “presque” un peu plus haut.

Il faut toutefois permettre à d’autres d’amener leurs langages.

1
2
3

// languages.properties
fr = FrenchNiceGuy
us = AmericanNotSoNiceGuy

J’ai donc un fichier properties pour lister les implémentations de mes langages. Maintenant, je dois mettre en place un mécanisme pour permettre d’utiliser nos différentes implémentations.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19

public class NiceGuyManager {
	private final Properties gentlemen = new Properties();
	public NiceGuyManager() throws IOException {
		this.gentlemen.load(NiceGuyManager.class.getResourceAsStream("/languages.properties"));
	}
	
	public boolean isLanguageSpoken(String language) {
		return gentlemen.containsKey(language);
	}
	
	public NiceGuy forLocale(String language)
			throws ClassNotFoundException, IllegalAccessException, InstantiationException { 
		if (!isLanguageSpOken(language)) {
			throw new ClassNotFoundException(String.format("The language %s is not recognized", language));
		}
		return (NiceGuy) Class.forName(gentlemen.get(language).toString()).newInstance();
	}

}

Et on remplace dans Main new NiceGuy("XX").sayHi() par manager.forLocale("XX"). Normalement, tous les tests sont bons. On a donc maintenant un état validé de notre comportement. Rajoutons quelques tests pour notre NiceGuyManager :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19

public class NiceGuyManagerTest {

	private NiceGuyManager manager;

	@Before
	public void setUp() throws Exception {
		this.manager = new NiceGuyManager();
	}

	@Test
	public void getFR() throws Exception {
		assertThat(manager.forLocale("fr")).isNotNull().isInstanceOf(FrenchNiceGuy.class);
	}

	@Test(expected = ClassNotFoundException.class)
	public void getUnknown() throws Exception {
		manager.forLocale("notdefined");
	}
}

Étape 5 : relecture par un très très bon

“Et sinon SPI tu connais ?”

On cherche, on trouve, on lit, on s’incline : Oui j’ai réinventé la roue dans l’étape 4. Donc on va faire du refactor. Pas de peur, grâce aux tests, on sait que l’on retrouvera au moins aussi bien.

On rajoute le langage dans NiceGuy: 

1
2
3
4

public interface NiceGuy {
	String sayHi();
	String getLocale();
}

On implémente la nouvelle méthode dans FrenchNiceGuy et AmericanNotSoNiceGuy. Les tests n’ont pas à bouger et sont encore valides. On voit là l’importance de ces tests, puisque nous avons “amélioré” nos implémentations sans contrarier le bon fonctionnement du code existant.

On réécrit NiceGuyManager pour utiliser le fonctionnement de SPI :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31

public class NiceGuyManager {
	private final Map<String, NiceGuy> gentlemen = new HashMap<String, NiceGuy>();

	public NiceGuyManager() {
		ServiceLoader<NiceGuy> gentlemenServiceLoader = ServiceLoader.load(NiceGuy.class);
        Iterator<NiceGuy> iterator = gentlemenServiceLoader.iterator();
        for (NiceGuy gentleman; iterator.hasNext(); ) {
            try {
                gentleman = iterator.next();
                gentlemen.put(gentleman.getLocale(), gentleman);
            } catch (ServiceConfigurationError error) {
            	// can be logged
            }
        }
	}
	
	public boolean isLanguageSpeaken(String language) {
		return gentlemen.containsKey(language);
	}
	
	public NiceGuy forLocale(String language)
			throws ClassNotFoundException {
		NiceGuy gentleman = gentlemen.get(language);
        if (gentleman == null) {
            throw new ClassNotFoundException(String.format("There is no class that can manage the language %s",
                    language));
        }
        return gentleman;
	}

}

et nous transformons le fichier properties en un fichier NiceGuy dans un dossier “META-INF/services”. On peut rejouer les tests et on s’aperçoit que nous n’avons là encore pas dégradé le fonctionnement de notre implémentation. Certes, nous aurions pu nous arrêter en étape 4, mais nous avons bénéficié d’un bon conseil et qui a potentiellement grandement amélioré le futur de notre application.

Conclusion

En partant d’un code moche et inutilisable sur le moyen terme, nous avons pu obtenir un code simple et efficace. Un brin de lecture, d’aide et c’est tout. Par principe, personne ne sait tout: travailler à plusieurs permet de combler certains manques.

D’autre part, nous avons :

  • écrit des tests, afin de garder un état valide de notre code,
  • utilisé un éditeur de code que l’on connait bien,
  • un SCM efficace, Git évidemment.

En un sens, il faut toujours se remettre en cause en informatique. En appliquant certaines pratiques de développement informatique, on peut rapidement s’améliorer. Vous pouvez également mettre en place des outils de relecture de code comme Gerrit ou des phases de Pair Programming. Ces éléments peuvent avoir un retour sur investissement (puisque certains ne comprennent que ça) très important, au même titre que plus de RAM, plus d’écran ou une formation…

Merci de m’avoir lu !

Références