15 - Refactoring legacy -- sans tout casser
Ce que tu vas apprendre
- Ce que "code legacy" veut vraiment dire (ce n'est pas une question d'age)
- Comment ajouter des tests sur du code non teste
- Le pattern Strangler Fig pour migrer progressivement
- Les feature flags comme filet de sécurité
- La regle du boy scout
- Quand ne pas refactorer
Prerequisites
Cet article fait suite a 14 - Techniques de refactoring. Toutes les techniques de l'article précédent s'appliquent ici, mais dans un contexte ou le code existant n'a pas de tests.
Michael Feathers definit le code legacy en une phrase : "Legacy code is code without tests." Pas du vieux code. Pas du code en COBOL. Du code sans tests. Un fichier TypeScript écrit la semaine dernière sans un seul test est du code legacy.
J'ai passe un an sur un monolithe Express de 80 000 lignes. Zero test. Chaque modification etait un pari. On deployait le vendredi (mauvaise idee) et on priait. Voici ce que j'ai appris sur comment s'en sortir.
Étape zero : ne pas tout reecrire
Le reflexe de beaucoup de devs face au legacy est de dire : "On jette tout et on recommence." C'est presque toujours une erreur. Joel Spolsky a écrit un article celebre la-dessus en 2000.
La reecriture complète :
- Prend deux a trois fois plus longtemps que prevu
- Reproduit les memes bugs que l'ancien système (parce que les specs sont dans le code)
- Ne livre rien aux utilisateurs pendant des mois
- Finit souvent par etre abandonnee
La bonne approche est incrementale. On ameliore le code existant morceau par morceau, avec un filet de sécurité.
Tests de caracterisation
Avant de toucher au code, il faut comprendre ce qu'il fait. Pas ce qu'il devrait faire -- ce qu'il fait réellement. Les tests de caracterisation capturent le comportement actuel, bugs inclus.
typescript// Le code legacy qu'on veut refactorer
function calculatePrice(
product: any,
quantity: any,
coupon: any
): number {
let price = product.price * quantity;
if (coupon) {
if (coupon.type === "percent") {
price = price - (price * coupon.value / 100);
} else {
price = price - coupon.value;
}
}
if (price < 0) price = 0;
// Bug connu : les taxes ne sont pas appliquees
// quand il y a un coupon. Mais 200 commandes
// sont passees comme ca. On ne corrige pas encore.
if (!coupon) {
price = price * 1.2;
}
return Math.round(price * 100) / 100;
}
Le test de caracterisation documente ce comportement tel quel :
typescriptdescribe("calculatePrice - characterization tests", () => {
test("sans coupon, applique la TVA", () => {
const product = { price: 100 };
expect(calculatePrice(product, 2, null)).toBe(240);
});
test("avec coupon pourcentage, pas de TVA (bug connu)", () => {
const product = { price: 100 };
const coupon = { type: "percent", value: 10 };
expect(calculatePrice(product, 2, coupon)).toBe(180);
});
test("avec coupon fixe, pas de TVA (bug connu)", () => {
const product = { price: 100 };
const coupon = { type: "fixed", value: 50 };
expect(calculatePrice(product, 1, coupon)).toBe(50);
});
test("le prix ne descend jamais sous zero", () => {
const product = { price: 10 };
const coupon = { type: "fixed", value: 500 };
expect(calculatePrice(product, 1, coupon)).toBe(0);
});
});
Ces tests ne disent pas si le code est correct. Ils disent ce qu'il fait maintenant. Si mon refactoring change un comportement, le test casse et je le sais immédiatement.
Le pattern Strangler Fig
Le nom vient du figuier etrangleur : une plante qui pousse autour d'un arbre existant et le remplace progressivement. En code, c'est pareil.
typescript// Etape 1 : on cree le nouveau module a cote de l'ancien
// ancien : src/services/payment.ts (500 lignes, pas de types)
// nouveau : src/services/payment-v2.ts (type, teste)
// Etape 2 : on redirige progressivement les appelants
class PaymentRouter {
constructor(
private legacy: LegacyPaymentService,
private modern: ModernPaymentService,
private featureFlags: FeatureFlags
) {}
async processPayment(order: Order): Promise<PaymentResult> {
if (this.featureFlags.isEnabled("new-payment-service")) {
return this.modern.process(order);
}
return this.legacy.process(order);
}
}
// Etape 3 : quand le nouveau service gere 100% du trafic,
// on supprime l'ancien
Le Strangler Fig marche parce qu'a chaque instant, le système fonctionne. Il n'y a pas de big bang. Pas de "on deploie tout en meme temps". Chaque étape est petite et reversible.
Feature flags pour la sécurité
Les feature flags permettent d'activer le nouveau code progressivement. D'abord pour 1% des utilisateurs, puis 10%, puis 50%, puis 100%.
typescript// Implementation simple de feature flags
interface FeatureFlags {
isEnabled(flag: string, context?: { userId?: string }): boolean;
}
class SimpleFeatureFlags implements FeatureFlags {
private flags: Map<string, {
enabled: boolean;
percentage?: number;
allowedUsers?: string[];
}> = new Map();
isEnabled(flag: string, context?: { userId?: string }): boolean {
const config = this.flags.get(flag);
if (!config || !config.enabled) return false;
if (config.allowedUsers?.length && context?.userId) {
return config.allowedUsers.includes(context.userId);
}
if (config.percentage !== undefined) {
const hash = this.hashUserId(context?.userId ?? "anonymous");
return (hash % 100) < config.percentage;
}
return true;
}
private hashUserId(userId: string): number {
let hash = 0;
for (const char of userId) {
hash = ((hash << 5) - hash) + char.charCodeAt(0);
hash |= 0;
}
return Math.abs(hash);
}
}
Si le nouveau code pose problème, un changement de configuration le désactivé. Pas de rollback, pas de redeploy.
Refactoring en petits pas
Le refactoring legacy se fait en étapes microscopiques. Chaque étape est committable et deployable.
Voici une sequence typique pour refactorer une fonction de 200 lignes :
Commit 1 : Ajouter des tests de caracterisation
Commit 2 : Extraire la validation dans validateInput()
Commit 3 : Extraire le calcul dans computeResult()
Commit 4 : Extraire le formatage dans formatOutput()
Commit 5 : Ajouter des types TypeScript aux parametres
Commit 6 : Remplacer les any par des types stricts
Commit 7 : Renommer les variables obscures
Commit 8 : Corriger le bug de TVA (maintenant que les tests couvrent le reste)
Huit commits. Chacun fait une seule chose. Chacun passe les tests. Si le commit 5 introduit un problème, on le revert sans perdre les commits 1 a 4.
La regle du boy scout
"Laisse le campement plus propre que tu ne l'as trouve." En code, ca veut dire : chaque fois que tu touches un fichier, ameliore-le un peu.
Tu corriges un bug dans une fonction ? Renomme une variable obscure au passage. Tu ajoutes une fonctionnalité ? Ajoute un test pour le code existant dans le meme fichier.
typescript// Avant (tu ouvres ce fichier pour ajouter un champ)
function getUser(id: any) {
const u = db.query("SELECT * FROM users WHERE id = " + id);
return u;
}
// Tu ajoutes ton champ ET tu ameliores un peu
function getUserById(id: string): Promise<User | null> {
const user = await db.query(
"SELECT * FROM users WHERE id = $1",
[id]
);
return user ?? null;
}
Injection SQL corrigee, types ajoutes, nom ameliore. En cinq minutes de travail supplementaire.
Le boy scout ne fait pas de grands refactorings. Il fait des micro-ameliorations continues. Sur six mois, l'effet cumule est massif.
Quand ne pas refactorer
Le refactoring n'est pas toujours la bonne réponse. Ne refactore pas :
- Du code que personne ne touche. S'il marche et que personne ne le modifie, laisse-le tranquille.
- Du code qu'on va supprimer. Si la fonctionnalité part dans deux sprints, ne perds pas de temps a la nettoyer.
- Du code critique en periode de gel. Avant un lancement, la stabilité bat la proprete.
- Sans tests. Le refactoring sans filet de sécurité n'est pas du refactoring, c'est de la roulette.
La question a se poser : "Est-ce que ce refactoring va faciliter un changement que je dois faire bientot ?" Si oui, fais-le. Si non, passe ton chemin.
Pour aller plus loin sur les stratégies de migration incrementale, retrouve des retours d'experience concrets sur paltemps.fr.
Résumé
- Le code legacy est du code sans tests, peu importe son age
- Ne reecris pas tout -- ameliore incrementalement
- Les tests de caracterisation capturent le comportement actuel, bugs inclus
- Le Strangler Fig remplace l'ancien code progressivement sans interruption
- Les feature flags permettent de déployer sans risque et de revenir en arriere
- Chaque commit de refactoring fait une seule chose et passe les tests
- La regle du boy scout : ameliore un peu chaque fichier que tu touches
- Ne refactore pas du code que personne ne touche ou qui va disparaître
Article précédent : 14 - Techniques de refactoring
Article suivant : 16 - Tests comme filet de sécurité