Les Change Preventers : ces code smells qui signalent un code résistant au changement
Du code facile à changer, épisode 3
Dans l’épisode précédent, j’ai présenté les Bloaters, ces smells qui signalent du code devenu trop gros.
Ils ont un avantage : ils se voient à la lecture. Une méthode de trois cents lignes, on la repère en faisant défiler l’écran.
Les Change Preventers sont plus discrets. On peut ouvrir chaque fichier du projet, les trouver tous courts et bien nommés, et ne rien remarquer. Ils ne se voient pas dans un fichier : ils se voient au moment où on modifie le code, ou quand on regarde son historique.
Tu as peut-être déjà vécu la scène. Une demande qui a l’air simple, “c’est juste un taux de TVA à ajouter”, et une pull request qui finit avec douze fichiers modifiés.
Comme dans les épisodes précédents, je pars du principe qu’un smell est un symptôme : il révèle un problème de conception, et il annonce un coût potentiel. Je vais donc te présenter chaque Change Preventer de la même façon : ce qu’on voit, le problème de conception en dessous, et ce que ça risque de coûter.
La famille compte trois membres. Les deux premiers sont le miroir l’un de l’autre, et le troisième est un cas particulier du deuxième.
Divergent Change : le carrefour
Le Divergent Change, c’est une classe qu’on modifie souvent, pour des raisons qui n’ont rien à voir entre elles.
La cause vient souvent d’un mélange de responsabilités dans une même classe. La classe qui génère un calcul, qui met en forme un document PDF et qui l’envoie directement par mail par exemple…
class Invoice {
constructor(
private lines: InvoiceLine[],
private customer: Customer,
private db: Database,
) {}
// Modifié quand une règle fiscale change
total(): number {
const net = this.lines.reduce((sum, line) => sum + line.price * line.quantity, 0);
return net * (1 + VAT_RATE);
}
// Modifié quand la charte graphique change
toPdf(): PdfFile {
const doc = new PdfDocument({ font: 'Helvetica', logo: 'logo-2024.png' });
// ... mise en page
return doc.render();
}
// Modifié quand le schéma de la base change
save(): void {
this.db.execute('INSERT INTO invoices (customer_id, total) VALUES (?, ?)', [
this.customer.id,
this.total(),
]);
}
}public class Invoice(List<InvoiceLine> lines, Customer customer, Database db)
{
// Modifié quand une règle fiscale change
public decimal Total()
{
var net = lines.Sum(line => line.Price * line.Quantity);
return net * (1 + Tax.VatRate);
}
// Modifié quand la charte graphique change
public PdfFile ToPdf()
{
var doc = new PdfDocument(font: "Helvetica", logo: "logo-2024.png");
// ... mise en page
return doc.Render();
}
// Modifié quand le schéma de la base change
public void Save()
{
db.Execute(
"INSERT INTO invoices (customer_id, total) VALUES (@id, @total)",
customer.Id, Total());
}
}Regarde les trois commentaires. Ils ne décrivent pas ce que fait la méthode, ils disent qui va demander à la modifier : la comptabilité pour le calcul, le marketing pour le PDF, l’équipe technique pour la base.
Si tu connais la Large Class, une classe qui fait trop de choses (je l’ai décrite dans l’épisode sur les Bloaters), tu vas me dire que ça y ressemble. C’est vrai, et les deux vont souvent ensemble. La différence, c’est l’endroit où on regarde.
La Large Class se mesure en nombre de lignes.
Le Divergent Change se lit dans l’historique : cette classe tient sur un écran, et elle apparaît quand même dans des commits qui n’ont aucun rapport entre eux.
Au fait, je t’ai déjà parlé de Barad-Dur ? C’est un outil open source qui permet d’analyser ton repo en se basant sur l’historique. Détecter les “hotspots”, c’est une des features principales.
Sinon avec une simple commande git tu peux analyser l’historique d’un fichier :
$ git log --oneline -- src/billing/invoice.ts
a1f3c2e Nouveau logo sur les factures
9b0d4e7 Arrondi de la TVA au centime
4c7e1a9 Migration vers PostgreSQL
e2d8f60 Police plus lisible dans le PDF
Quatre commits, trois sujets différents.
Le marketing demande : “on change le logo sur les factures”.
C’est une modification de mise en page, à faire dans le fichier qui calcule la TVA.
Pour la tester, il faut construire une facture complète, avec ses lignes, son client et une base de données. Et si quelqu’un travaille au même moment sur une règle fiscale, on se retrouve à deux dans le même fichier.
Le problème de conception : des responsabilités qui évoluent pour des raisons différentes sont rassemblées dans la même classe.
Le coût potentiel : un risque de régressions, puisqu’une modification de mise en page peut casser un calcul, et une compréhension difficile.
Le remède s’appelle Extract Class, comme pour la Large Class. Ce qui change, c’est le critère de découpage : on sépare selon les raisons de changer.
// Change quand une règle fiscale change
export function totalOf(lines: InvoiceLine[]): number {
const net = lines.reduce((sum, line) => sum + line.price * line.quantity, 0);
return net * (1 + VAT_RATE);
}
// Change quand la charte graphique change
export class InvoicePdf {
render(invoice: Invoice): PdfFile {
const doc = new PdfDocument({ font: 'Helvetica', logo: 'logo-2024.png' });
// ... mise en page
return doc.render();
}
}
// Change quand le schéma de la base change
export class InvoiceRepository {
constructor(private db: Database) {}
save(invoice: Invoice): void {
this.db.execute('INSERT INTO invoices (customer_id, total) VALUES (?, ?)', [
invoice.customer.id,
totalOf(invoice.lines),
]);
}
}// Change quand une règle fiscale change
public static class InvoiceTotal
{
public static decimal Of(List<InvoiceLine> lines)
{
var net = lines.Sum(line => line.Price * line.Quantity);
return net * (1 + Tax.VatRate);
}
}
// Change quand la charte graphique change
public class InvoicePdf
{
public PdfFile Render(Invoice invoice)
{
var doc = new PdfDocument(font: "Helvetica", logo: "logo-2024.png");
// ... mise en page
return doc.Render();
}
}
// Change quand le schéma de la base change
public class InvoiceRepository(Database db)
{
public void Save(Invoice invoice)
{
db.Execute(
"INSERT INTO invoices (customer_id, total) VALUES (@id, @total)",
invoice.Customer.Id, InvoiceTotal.Of(invoice.Lines));
}
}Le changement de logo ne concerne plus qu’un fichier, et ce fichier ne contient que de la mise en page.
Shotgun Surgery : un tir, vingt impacts
La Shotgun Surgery, c’est l’inverse : un seul changement, qui oblige à faire beaucoup de petites modifications dans beaucoup de fichiers.
Le nom vient du fusil de chasse : une seule cartouche, et des impacts un peu partout.
J’ai déjà utilisé un exemple dans l’épisode 1, avec le transporteur DHL qu’il fallait ajouter à quatre endroits.
En voici un autre, sans aucun switch, parce que ce smell ne se limite pas aux conditions.
Je reste dans la facturation. Dans la section précédente, le calcul de la TVA de la facture a gagné son propre fichier. Mais la facture n’est pas le seul endroit où on calcule de la TVA.
// tax.ts
export const VAT_RATE = 0.2;
// cart/cart.ts : le total du panier
const total = net * (1 + VAT_RATE);
// billing/invoice-total.ts : le total de la facture
const invoiceTotal = net * (1 + VAT_RATE);
// billing/invoice-pdf.ts : la ligne de TVA du PDF
doc.text(`TVA (${VAT_RATE * 100} %) : ${format(net * VAT_RATE)}`);
// catalog/product-card.ts : le prix affiché sur la fiche produit
const displayedPrice = product.price * (1 + VAT_RATE);// Tax.cs
public static class Tax
{
public const decimal VatRate = 0.20m;
}
// Cart/Cart.cs : le total du panier
var total = net * (1 + Tax.VatRate);
// Billing/InvoiceTotal.cs : le total de la facture
var invoiceTotal = net * (1 + Tax.VatRate);
// Billing/InvoicePdf.cs : la ligne de TVA du PDF
doc.AddLine($"TVA ({Tax.VatRate:P0}) : {net * Tax.VatRate:F2} €");
// Catalog/ProductCard.cs : le prix affiché sur la fiche produit
var displayedPrice = product.Price * (1 + Tax.VatRate);Le panier, la facture, le PDF et la fiche produit calculent chacun de la TVA.
Remarque que personne n’a écrit 0.2 en dur.
Le taux est une constante partagée, donc on ne parle pas ici de nombres magiques.
Ce qui est recopié, c’est le calcul : la façon de passer d’un prix hors taxes à un prix TTC.
Le métier annonce : “on ajoute des livres au catalogue, et leur TVA est à 5,5 %”.
La constante ne suffit plus : il n’y a plus un taux, il y en a un par catégorie de produit. Il faut donc reprendre les quatre calculs, et chacun change à sa façon. Le panier doit regarder la catégorie de chaque article. Le PDF doit afficher une ligne de TVA par taux. La fiche produit doit connaître la catégorie de son produit.
Et il reste tous les endroits que je n’ai pas montrés : les avoirs, l’export comptable, les emails de confirmation. Si on en oublie un, la fiche produit affiche un livre à un prix, et le panier en demande un autre.
Le problème de conception : une décision du métier, ici le calcul de la TVA, est répartie dans le code. C’est une duplication de connaissance, celle dont je parle dans l’article sur DRY.
Le coût potentiel : de multiples changements, et un risque de régressions chaque fois qu’on en oublie un.
Dans la section précédente, une seule classe changeait pour trois raisons. Ici, c’est le miroir : une seule raison fait changer quatre fichiers.
Pour le remède, Martin Fowler parle de Move Method et de Move Field : on regroupe ce qui est éparpillé vers un seul endroit, et les autres fichiers pointent vers lui.
// tax/vat.ts
type ProductCategory = 'standard' | 'book';
const RATES: Record<ProductCategory, number> = { standard: 0.2, book: 0.055 };
export const vat = {
rateOf: (category: ProductCategory): number => RATES[category],
withTax: (net: number, category: ProductCategory): number => net * (1 + RATES[category]),
};
// cart/cart.ts
const total = items.reduce((sum, item) => sum + vat.withTax(item.price * item.quantity, item.category), 0);
// catalog/product-card.ts
const displayedPrice = vat.withTax(product.price, product.category);// Tax/Vat.cs
public enum ProductCategory { Standard, Book }
public static class Vat
{
private static readonly Dictionary<ProductCategory, decimal> Rates = new()
{
[ProductCategory.Standard] = 0.20m,
[ProductCategory.Book] = 0.055m,
};
public static decimal RateOf(ProductCategory category) => Rates[category];
public static decimal WithTax(decimal net, ProductCategory category) => net * (1 + Rates[category]);
}
// Cart/Cart.cs
var total = items.Sum(item => Vat.WithTax(item.Price * item.Quantity, item.Category));
// Catalog/ProductCard.cs
var displayedPrice = Vat.WithTax(product.Price, product.Category);Le jour où un troisième taux arrive, il y a un seul fichier à ouvrir. Les autres fichiers ne savent plus comment la TVA se calcule : ils savent seulement à qui le demander.
Comment repérer ce smell avant le jour du changement ? Une piste : regarder le nombre de fichiers dans tes dernières pull requests. Si une petite demande en touche régulièrement une dizaine, regarde lesquels reviennent ensemble.
C’est ce que mesure le couplage temporel dans Barad-Dur : les fichiers qui changent régulièrement ensemble, alors qu’il n’y a pas de lien apparent entre eux.
Parallel Inheritance Hierarchies : les hiérarchies jumelles
Des Parallel Inheritance Hierarchies, ce sont des hiérarchies de classes qui grandissent ensemble : chaque fois qu’on ajoute une classe dans l’une, il faut en ajouter une dans l’autre.
Martin Fowler le présente comme un cas particulier de la Shotgun Surgery.
Je reprends l’exemple des transporteurs que j’ai utilisé dans l’épisode 1.
Le transporteur y était testé dans trois switch : un pour les frais de port, un pour le délai de livraison, un pour le libellé.
Un réflexe fréquent consiste à remplacer chaque switch par sa propre hiérarchie de classes, et on obtient ceci.
interface FeeCalculator { fee(weightKg: number): number }
class ColissimoFeeCalculator implements FeeCalculator { fee(weightKg: number) { return 4.9 + weightKg * 0.5; } }
class ChronopostFeeCalculator implements FeeCalculator { fee(weightKg: number) { return 9.9 + weightKg * 0.8; } }
class RelayFeeCalculator implements FeeCalculator { fee() { return 3.5; } }
interface DelayEstimator { days(): number }
class ColissimoDelayEstimator implements DelayEstimator { days() { return 3; } }
class ChronopostDelayEstimator implements DelayEstimator { days() { return 1; } }
class RelayDelayEstimator implements DelayEstimator { days() { return 4; } }
interface LabelPrinter { label(): string }
class ColissimoLabelPrinter implements LabelPrinter { label() { return 'Colissimo'; } }
class ChronopostLabelPrinter implements LabelPrinter { label() { return 'Chronopost 24h'; } }
class RelayLabelPrinter implements LabelPrinter { label() { return 'Point relais'; } }public interface IFeeCalculator { decimal Fee(decimal weightKg); }
public class ColissimoFeeCalculator : IFeeCalculator { public decimal Fee(decimal weightKg) => 4.9m + weightKg * 0.5m; }
public class ChronopostFeeCalculator : IFeeCalculator { public decimal Fee(decimal weightKg) => 9.9m + weightKg * 0.8m; }
public class RelayFeeCalculator : IFeeCalculator { public decimal Fee(decimal weightKg) => 3.5m; }
public interface IDelayEstimator { int Days(); }
public class ColissimoDelayEstimator : IDelayEstimator { public int Days() => 3; }
public class ChronopostDelayEstimator : IDelayEstimator { public int Days() => 1; }
public class RelayDelayEstimator : IDelayEstimator { public int Days() => 4; }
public interface ILabelPrinter { string Label(); }
public class ColissimoLabelPrinter : ILabelPrinter { public string Label() => "Colissimo"; }
public class ChronopostLabelPrinter : ILabelPrinter { public string Label() => "Chronopost 24h"; }
public class RelayLabelPrinter : ILabelPrinter { public string Label() => "Point relais"; }Neuf classes, et un indice dans leurs noms : les mêmes préfixes reviennent dans chaque hiérarchie.
Le métier annonce : “on ajoute DHL”.
Il faut créer trois classes, dans trois dossiers, et mettre à jour les trois endroits qui choisissent la bonne classe.
Les switch ont disparu, mais la Shotgun Surgery est toujours là.
On a retiré le symptôme sans traiter le problème.
Le problème de conception : un même concept du métier, le transporteur, est découpé selon ce qu’on en fait, alors que tout ce qui le concerne change en même temps.
Le coût potentiel : de multiples changements à chaque nouveau cas.
Le remède, c’est de regrouper : un seul objet par transporteur, qui porte tout ce qu’on a besoin de savoir sur lui.
type CarrierId = 'colissimo' | 'chronopost' | 'relay';
interface Carrier {
label: string;
delayDays: number;
fee(weightKg: number): number;
}
const carriers: Record<CarrierId, Carrier> = {
colissimo: { label: 'Colissimo', delayDays: 3, fee: (kg) => 4.9 + kg * 0.5 },
chronopost: { label: 'Chronopost 24h', delayDays: 1, fee: (kg) => 9.9 + kg * 0.8 },
relay: { label: 'Point relais', delayDays: 4, fee: () => 3.5 },
};public enum CarrierId { Colissimo, Chronopost, Relay }
public sealed record Carrier(string Label, int DelayDays, Func<decimal, decimal> Fee);
public static class Carriers
{
public static readonly IReadOnlyDictionary<CarrierId, Carrier> All = new Dictionary<CarrierId, Carrier>
{
[CarrierId.Colissimo] = new("Colissimo", 3, kg => 4.9m + kg * 0.5m),
[CarrierId.Chronopost] = new("Chronopost 24h", 1, kg => 9.9m + kg * 0.8m),
[CarrierId.Relay] = new("Point relais", 4, _ => 3.5m),
};
}Ajouter DHL revient à ajouter une ligne.
En TypeScript, le compilateur aide : si tu ajoutes 'dhl' au type sans ajouter la ligne correspondante, le code ne compile plus.
En C#, le dictionnaire n’offre pas cette garantie. Pour garantir l’exhaustivité, on peut écrire un test qui parcourt toutes les valeurs de l’énumération.
Deux smells qui tirent en sens inverse
Le Divergent Change pousse à découper. La Shotgun Surgery pousse à regrouper.
Si on corrige l’un de façon mécanique, on peut fabriquer l’autre. En découpant la facture en douze petites classes, le jour où elle gagne un champ, il faut ouvrir douze fichiers.
En factorisant les douze petites classes en une, on risque d’avoir un “god object” qui deviendra un aimant à responsabilités, créant le smell divergent change.
Ce n’est pas forcément une contradiction. Dans les deux cas, je me pose la même question : est-ce que les choses qui changent ensemble sont rangées ensemble ?
Si la réponse est oui, peu importe que la classe soit grande ou petite.
Je n’ai pas de règle plus générale à proposer ici, c’est un des nombreux cas où la meilleure réponse que j’aie à donner est le fameux : “ça dépend”.
Attention aux faux positifs
Un smell signale un problème potentiel. Voici quelques cas qui ressemblent à un Change Preventer, alors que tout va bien.
- Le fichier qui change souvent, pour une seule raison. Un module de tarification, dans une entreprise qui revoit ses prix chaque semaine, sera modifié chaque semaine. Ce n’est pas un Divergent Change : la raison reste la même.
- Le changement qui traverse les couches. Ajouter un champ touche le formulaire, l’API, l’entité et la migration. Certes, ça fait quatre fichiers. Mais chacun fait un travail différent, et aucun ne contient la copie d’une règle.
- Le renommage. Renommer une classe peut modifier quarante fichiers. C’est mécanique, l’outil le fait à ta place, et ça ne dit rien de la conception.
- Les fichiers qui vont par deux. Une classe et son test, une entité (au sens entity de Entity Framework par exemple) et sa migration : il est normal qu’ils changent ensemble.
La question à se poser n’est donc pas “combien de fichiers j’ai modifiés ?” mais “est-ce que j’ai dû retrouver la même décision à plusieurs endroits ?”.
Et avec un agent IA ?
Dans l’épisode 1, j’expliquais qu’un agent retrouve mal toutes les copies d’une même décision. La Shotgun Surgery, c’est exactement cette situation.
Un agent modifie bien un endroit précis. Il ne retrouve pas forcément les trois autres copies du calcul, surtout quand elles ne se ressemblent pas.
Le Divergent Change lui pose un autre problème. Pour changer un logo, il charge dans son contexte un fichier qui parle aussi de TVA et de base de données, et il peut y toucher sans qu’on le lui demande.
Dans les deux cas, la relecture coûte plus cher : une pull request de vingt fichiers demande plus d’attention qu’une pull request de deux.
En résumé
| Smell | Le problème de conception | Le coût potentiel | Le remède principal |
|---|---|---|---|
| Divergent Change | Plusieurs raisons de changer dans une classe | Risque de régressions, compréhension difficile | Extract Class |
| Shotgun Surgery | Une décision du métier répartie dans le code | Multiples changements, risque de régressions | Move Method, Move Field |
| Parallel Inheritance Hierarchies | Un concept découpé en hiérarchies jumelles | Multiples changements | Regrouper dans un seul objet |
Dans l’épisode 1 où je décris les raisons d’un changement coûteux, j’avais rangé toute cette famille du côté des multiples changements.
C’est vrai pour deux smells sur trois. Le Divergent Change, lui, annonce surtout un risque de régressions.
La suite
Dans le prochain épisode, je vais m’attaquer aux Object-Orientation Abusers.
Le nom est barbare, mais tu en as déjà croisé : le switch qui se répète en fait partie.
Tu as envie de lire la suite ? Abonne-toi à mon flux RSS pour être tenu au courant en direct de mes publications !
Tu veux creuser ces sujets avec ton équipe ? J’interviens en Brown Bag Lunch dans vos locaux, ou par mail à edouard@lacrafterie.tech.