Legacy code – Se défaire des dépendances statiques

Depuis quelques années, et particulièrement ces derniers mois, le mouvement Software Craftsmanship gagne du terrain et convainc de plus en plus de développeurs et de DSI.
Si les valeurs qu’il soutient sont relativement bien comprises, une interrogation revient régulièrement : « Comment appliquer tout cela sur le legacy code? »

Lors de la dernière réunion du Paris Software Craftsmanship, cette question est revenue à l’ordre du jour et nous a donné l’idée de cet article.
L’objectif est de vous présenter des techniques et astuces pas nécessairement nouvelles, connues de certains mais au combien utiles et adaptées à la réalité quotidienne du legacy code.

Au menu du jour, les dépendances fortes qui se rapportent souvent à des appels statiques.

Avant de commencer

Pour étayer nos propos, nous allons nous appuyer sur le code d’un exercice de refactoring proposé par Sandro Mancuso, il y a quelques années.
N’hésitez pas à consulter cette vidéo qui détaille d’autres concepts non abordés dans cet article.

Vous retrouverez tous les exemples mentionnés sur notre repository GitHub, où nous avons forké le projet original.
Chaque jalon important correspondra à un tag. Enfin, nous nous concentrerons sur le langage Java mais sachez que d’autres versions sont disponibles (C++, C#, Php, Python et Scala).

Le legacy code

Entrons dans le vif du sujet et découvrons ce qui va constituer notre legacy code :

[java]public class TripService {

public List<Trip> getTripsByUser(User user) throws UserNotLoggedInException {
List<Trip> tripList = new ArrayList<Trip>();
User loggedUser = UserSession.getInstance().getLoggedUser();
boolean isFriend = false;
if (loggedUser != null) {
for (User friend : user.getFriends()) {
if (friend.equals(loggedUser)) {
isFriend = true;
break;
}
}
if (isFriend) {
tripList = TripDAO.findTripsByUser(user);
}
return tripList;
} else {
throw new UserNotLoggedInException();
}
}

}[/java]

Il s’agit d’un service métier dont la responsabilité est de traiter des voyages.
La méthode présentée ici se charge :

• de récupérer les voyages d’un utilisateur si celui-ci est ami avec l’utilisateur connecté ;
• de gérer les exceptions à cette règle (pas d’utilisateur connecté, utilisateurs qui ne sont pas amis).

Ce service n’est couvert par aucun test unitaire et c’est bien évidemment là l’objectif premier de l’exercice proposé par Sandro.
Avant de démarrer, prenons le temps de rappeler quelques fondamentaux.

Quelques rappels

Il convient de s’entendre sur une définition du legacy code. Nous proposons la suivante, communément admise mais peut être incomplète selon les contextes.

Un legacy code est un code non testé.

D’autre part, et comme le rappelle Sandro, aucun code de production ne peut être modifié s’il n’est pas couvert par des tests unitaires.
Autrement dit, pas de refactoring sans filet de sécurité. Seules les actions automatisées par les IDE sont admises dans le cas où elles sont strictement nécessaires pour écrire un test, ce que nous allons voir dans la suite de cet article.

Allez, trêve de bavardages, c’est parti!

Isolation de la thin layer

Notre but est donc de mettre en place les tests unitaires sur notre legacy code pour se permettre de le modifier par la suite.
Une technique classique est de tester en premier lieu les branches de code les moins profondes, c’est à dire celles situées le plus à gauche.
La raison à cela est simple : plus une branche est profonde, plus elle nécessite de préparation pour y accéder.

Ainsi, le premier test que nous allons chercher à écrire est celui où aucun utilisateur n’est connecté (une exception est alors attendue).
Cependant, quelque chose nous empêche déjà d’y parvenir :

[java]User loggedUser = UserSession.getInstance().getLoggedUser();[/java]

Notre code fait usage d’un appel statique pour récupérer la session puis l’utilisateur connecté à partir de cette dernière. C’est ce que nous appelons une dépendance forte.
Cette dernière nous dérange car nous n’avons pas la main sur son comportement.

Idéalement, nous souhaiterions simuler ce dernier.  Une première solution consiste à utiliser un framework comme PowerMock. Nous vous le déconseillons fortement.
D’une part, ce dernier est bien plus lent qu’un framework de tests unitaires classique. D’autre part, vous ne feriez rien d’autre que cacher la poussière sous le tapis, sans corriger le réel problème de votre code.

En lieu et place, nous allons extraire cette dépendance dans une méthode. Pour ce faire, nous nous appuyons sur notre IDE (rappelez-vous la règle énoncée plus haut).

[java]protected User getLoggedUser() {
return UserSession.getInstance().getLoggedUser();
}[/java]

Notre expression initiale a désormais la forme suivante :

[java]User loggedUser = getLoggedUser();[/java]

Le fait d’avoir déclaré notre méthode avec la visibilité protected va nous permettre de proposer une nouvelle implémentation dans notre classe de tests :

[java]public class TripServiceTest {
 
private final TripService tripService = new TestableTripService();

private class TestableTripService extends TripService {

@Override
protected User getLoggedUser() {
return null; // Desired behavior
}
}
}[/java]

Nous avons maintenant la main sur le comportement de notre dépendance, sans pour autant avoir pris le risque de modifier le code de production.
Le test que nous cherchons à écrire est maintenant trivial :

[java]@Test
public void should_throw_exception_when_no_logged_in_user() {

// Arrange
final User unusedUser = null;

// Act
catchException(tripService).getTripsByUser(unusedUser);

// Assert
assertThat(caughtException()).isInstanceOf(UserNotLoggedInException.class);
}[/java]

Note : nous faisons ici usage des librairies AssertJ et Google Catch Exception qui apportent une touche de lisibilité intéressante mais ne sont pas liées aux techniques présentées.

Nous traitons de la même manière l’autre dépendance forte de notre code. La ligne…

[java]tripList = TripDAO.findTripsByUser(user);[/java]

… est remplacée par

[java]protected List<Trip> tripsByUser(User user) {
return TripDAO.findTripsByUser(user);
}[/java]

[java]tripList = tripsByUser(user);[/java]

L’ensemble constitué par ces méthodes protégées est appelée thin layer. Il s’agit de la fine couche d’interface entre votre code et ses dépendances.
L’isoler de cette manière vous permet de tester l’ensemble de votre legacy code (voir le résultat obtenu à cette étape, au niveau du tag thin-layer).

En revanche, ce n’est pas une fin en soi. Sa présence n’a pour but que de vous permettre de tester votre code.
C’est la seule raison pour laquelle la visibilité protected est acceptable. Il vous faut donc nécessairement aller plus loin.

Note: Nous en profitons pour vous rappeler que vous ne devriez jamais changer la visibilité d’un élément (classe, méthode, etc.) dans l’unique but de le tester.

Explicitation des dépendances

Si nous nous sommes donné tant de mal à tester notre legacy code, c’est que nous souhaitons l’améliorer.
Pour rappel, voici sa forme actuelle.

[java]public class TripService {

public List<Trip> getTripsByUser(User user) throws UserNotLoggedInException {
List<Trip> tripList = new ArrayList<Trip>();
User loggedUser = getLoggedUser();
boolean isFriend = false;
if (loggedUser != null) {
for (User friend : user.getFriends()) {
if (friend.equals(loggedUser)) {
isFriend = true;
break;
}
}
if (isFriend) {
tripList = tripsByUser(user);
}
return tripList;
} else {
throw new UserNotLoggedInException();
}
}

protected List<Trip> tripsByUser(User user) {
return TripDAO.findTripsByUser(user);
}

protected User getLoggedUser() {
return UserSession.getInstance().getLoggedUser();
}

}[/java]

Nous allons nous concentrer sur nos dépendances qui, bien qu’étant maintenant simulables, restent fortement couplées à notre code.
Pire, elles ne sont pas explicites. Autrement dit, si l’on souhaite créer ce service, nous n’avons pas connaissance de ses dépendances et il est impossible de lui fournir.

[java]new TripService(); // No dependency expected[/java]

D’autre part, cela introduit une responsabilité qui ne lui est pas inhérente (voir Single Responsibility Principle). Ce service traite de voyages et ne devrait en aucun cas savoir comment récupérer la session courante.
C’est un anti-pattern extrêmement courant qui réduit considérablement la testabilité du code.

De ce fait, notre nouveau but est maintenant d’expliciter ces deux dépendances.

Explicitation paramétrique

Commençons par la dépendance vers la classe UserSession :

[java]UserSession.getInstance().getLoggedUser()[/java]

Cette dernière ne sert qu’à récupérer l’utilisateur connecté dans une seule et unique méthode.
On devine donc que l’on peut lui passer directement. Il nous faut donc en changer la signature. Pour minimiser les risques, faisons-le à travers notre IDE.

Note : Nous entrons ici dans une phase de  refactoring  qui comporte un léger risque puisque l’on modifie la signature d’une méthode.
Certes, notre IDE nous y aide en modifiant tous les appels à cette dernière, mais cela n’est pas suffisant.
Il faut avoir une vue complète de ces derniers pour valider par la suite que la bonne valeur est transmise. La quantité d’appels à modifier est un bon indicateur du risque pris. Afin de le minimiser, nous vous conseillons de mettre un espace comme valeur par défaut du paramètre, afin de générer une erreur syntaxique qui sera nécessairement détectée à la compilation. Ainsi, vous serez obligé de valider les appels modifiés.

Notre méthode devient donc :

[java]public List<Trip> getTripsByUser(User user, User loggedInUser) throws UserNotLoggedInException[/java]

Après avoir mis à jour ses appels (dans notre cas, cantonnés à nos TU) et vérifié que les tests passent toujours au vert, nous pouvons modifier notre méthode pour faire usage du nouveau paramètre :

[java]public List<Trip> getTripsByUser(User user, User loggedInUser) throws UserNotLoggedInException {
List<Trip> tripList = new ArrayList<Trip>();
boolean isFriend = false;
if (loggedInUser != null) {
for (User friend : user.getFriends()) {
if (friend.equals(loggedInUser)) {
isFriend = true;
break;
}
}
if (isFriend) {
tripList = tripsByUser(user);
}
return tripList;
} else {
throw new UserNotLoggedInException();
}
}[/java]

Si les tests passent toujours, la méthode de la thin layer peut être supprimée.

Explicitation par constructeur

Reste alors l’autre dépendance vers TripDAO.

[java]TripDAO.findTripsByUser(user)[/java]

Un premier problème est ici visible : il s’agit d’un appel statique direct. Dès lors, si l’on explicite notre dépendance pour obtenir une instance de classe, nous n’aurons toujours que la possibilité de faire cet appel.
Il nous faudrait donc enlever le caractère statique de cette méthode. Pour le faire de manière sécurisée, il faut :

• s’assurer que cette méthode est bien testée ;
• créer une méthode d’instance qui fait appel à la méthode statique à remplacer et la tester également.

[java]public List<Trip> tripsOf(User user) {
return TripDAO.findTripsByUser(user);
}[/java]

Cela permet de migrer un à un les appels. Une fois la migration vers la méthode d’instance terminée, la méthode statique peut être supprimée.
Mais avant cela, il nous faut nous attaquer à l’explicitation de notre dépendance. Il nous faut fournir celle-ci lors de la construction du service.

Note : Nous vous passons les détails quant à la marche à suivre pour le faire de manière sécurisée ; la vidéo de Sandro Mancuso le détaille parfaitement.

Au final, la dépendance est explicitée dans le constructeur de la classe :

[java]public class TripService {

private final TripDAO tripDAO;

public TripService(TripDAO tripDAO) {
this.tripDAO = tripDAO;
}
}[/java]

Et notre méthode ne fait plus usage de l’appel statique :

[java]public List<Trip> getTripsByUser(User user, User loggedInUser) throws UserNotLoggedInException {
List<Trip> tripList = new ArrayList<Trip>();
boolean isFriend = false;
if (loggedInUser != null) {
for (User friend : user.getFriends()) {
if (friend.equals(loggedInUser)) {
isFriend = true;
break;
}
}
if (isFriend) {
tripList = tripDAO.tripsOf(user);
}
return tripList;
} else {
throw new UserNotLoggedInException();
}
}[/java]

Le résultat final est accessible par le tag HEAD.

Note : Le code obtenu à cet instant n’est bien sûr pas optimal car là n’était pas l’objectif de cet article.
Vous pouvez consulter la vidéo de résolution du kata ou bien la branche java-solution pour avoir une idée de ce qui peut être obtenu.

Conclusion

Par cet article, nous vous avons montré comment vous défaire de vos dépendances statiques de manière sécurisée.
Nous avons d’abord vu comment isoler une thin layer vous permettant ainsi de tester l’ensemble de votre legacy code.

Nous avons ensuite vu comment expliciter vos dépendances, que ce soit au travers d’un paramètre ou d’une instance de classe.
Votre service :

• est dorénavant clair pour quiconque veut l’utiliser ; plus de mauvaises surprises !
• permet d’injecter des implémentations alternatives si nécessaire; sa testabilité n’en est que renforcée.

Si vous souhaitez creuser ce sujet plus en détails, nous vous recommandons ce guide produit par Google.

Enfin, sachez que l’isolation de la thin layer est également applicable à la création d’objets ou à des appels de méthodes finales, l’article ayant pour sa part couvert les appels statiques.