13 - Code smells -- les signes que le code a besoin de refactoring
Ce que tu vas apprendre
- Les code smells les plus frequents en TypeScript avec des exemples concrets
- Comment détecter chaque smell dans ton code
- La correction adaptee a chaque situation
- Faire la différence entre un smell réel et une fausse alerte
Prerequisites
Cet article fait suite a 12 - Abstractions prematurees vs tardives. Les concepts de SOLID et de complexité cyclomatique seront références.
Martin Fowler et Kent Beck ont popularise le terme "code smell" dans le livre Refactoring (1999). Un smell n'est pas un bug. Le code marche. Mais quelque chose sent mauvais -- un signe que la conception a un problème. Comme une fuite de gaz : tu ne la vois pas, mais si tu l'ignores assez longtemps, ca explose.
Voici les smells que je rencontre le plus souvent en TypeScript, avec un exemple et un fix pour chacun.
Long Method
Une fonction qui dépassé 30 lignes merite généralement d'etre decoupee. Au-dela de 50, c'est presque toujours un problème.
typescript// Smell : 60 lignes qui font validation + calcul + formatage
async function processOrder(order: Order): Promise<OrderResult> {
// 15 lignes de validation
if (!order.items.length) { /* ... */ }
if (!order.userId) { /* ... */ }
// ...
// 20 lignes de calcul
let subtotal = 0;
for (const item of order.items) { /* ... */ }
const tax = subtotal * 0.2;
// ...
// 25 lignes de formatage et persistence
const invoice = { /* ... */ };
await db.save(invoice);
// ...
}
// Fix : trois fonctions courtes
async function processOrder(order: Order): Promise<OrderResult> {
validateOrder(order);
const totals = calculateTotals(order);
return await saveAndFormat(order, totals);
}
God Object
Une classe qui fait tout. Elle connaît tout le monde, tout le monde la connaît. Impossible a tester en isolation.
typescript// Smell : la classe qui gere tout
class ApplicationManager {
private db: Database;
private cache: Cache;
private mailer: Mailer;
private logger: Logger;
private queue: Queue;
createUser() { /* ... */ }
deleteUser() { /* ... */ }
sendEmail() { /* ... */ }
processPayment() { /* ... */ }
generateReport() { /* ... */ }
clearCache() { /* ... */ }
// 40 autres methodes
}
// Fix : decouper par responsabilite (voir SRP dans l'article SOLID)
class UserService { /* create, delete, update */ }
class PaymentService { /* process, refund */ }
class ReportService { /* generate, export */ }
Feature Envy
Une méthode qui utilise plus les donnees d'un autre objet que les siennes.
typescript// Smell : cette methode de OrderPrinter envie les donnees de Order
class OrderPrinter {
print(order: Order): string {
const subtotal = order.items.reduce(
(sum, item) => sum + item.price * item.quantity, 0
);
const tax = subtotal * order.taxRate;
const total = subtotal + tax - order.discount;
return `Total: ${total}`;
}
}
// Fix : le calcul appartient a Order
class Order {
get subtotal(): number {
return this.items.reduce(
(sum, item) => sum + item.price * item.quantity, 0
);
}
get total(): number {
return this.subtotal + this.subtotal * this.taxRate - this.discount;
}
}
class OrderPrinter {
print(order: Order): string {
return `Total: ${order.total}`;
}
}
Primitive Obsession
Utiliser des types primitifs là où un type domaine serait plus sur et plus expressif.
typescript// Smell : des strings partout
function createUser(
email: string,
phone: string,
zipCode: string,
role: string
): User { /* ... */ }
// Rien n'empeche : createUser("75001", "admin", "hello", "foo@bar.com")
// Fix : des types dedies
type Email = string & { readonly __brand: "Email" };
type Phone = string & { readonly __brand: "Phone" };
type ZipCode = string & { readonly __brand: "ZipCode" };
type Role = "admin" | "editor" | "viewer";
function createEmail(raw: string): Email {
if (!raw.includes("@")) throw new Error("Email invalide");
return raw as Email;
}
function createUser(
email: Email,
phone: Phone,
zipCode: ZipCode,
role: Role
): User { /* ... */ }
Le type branding en TypeScript empeche de passer un ZipCode là où on attend un Email. Le compilateur attrape l'erreur. On en parle aussi dans l'article sur la programmation defensive.
Data Clumps
Des groupes de donnees qui voyagent toujours ensemble mais ne sont pas dans un objet.
typescript// Smell : ces trois parametres apparaissent ensemble partout
function createEvent(
startDate: Date, endDate: Date, timezone: string
): Event { /* ... */ }
function isOverlapping(
startDate1: Date, endDate1: Date, timezone1: string,
startDate2: Date, endDate2: Date, timezone2: string
): boolean { /* ... */ }
// Fix : un objet qui regroupe les donnees liees
interface DateRange {
start: Date;
end: Date;
timezone: string;
}
function createEvent(range: DateRange): Event { /* ... */ }
function isOverlapping(a: DateRange, b: DateRange): boolean { /* ... */ }
Switch Statements repetes
Un switch sur le meme type qui apparaît dans plusieurs fonctions.
typescript// Smell : le meme switch dans trois fonctions
function getIcon(status: Status): string {
switch (status) {
case "active": return "check";
case "pending": return "clock";
case "error": return "alert";
}
}
function getColor(status: Status): string {
switch (status) {
case "active": return "green";
case "pending": return "yellow";
case "error": return "red";
}
}
// Fix : regrouper dans une map
const statusConfig: Record<Status, { icon: string; color: string }> = {
active: { icon: "check", color: "green" },
pending: { icon: "clock", color: "yellow" },
error: { icon: "alert", color: "red" },
};
function getIcon(status: Status): string {
return statusConfig[status].icon;
}
Speculative Generality
Du code écrit pour des cas qui n'existent pas encore. On en a parle dans l'article sur YAGNI, mais ca merite un rappel ici.
typescript// Smell : un systeme de plugins pour un seul plugin
interface Plugin {
name: string;
version: string;
init(): Promise<void>;
execute(context: PluginContext): Promise<PluginResult>;
destroy(): Promise<void>;
}
class PluginManager {
private plugins: Map<string, Plugin> = new Map();
register(plugin: Plugin): void { /* ... */ }
unregister(name: string): void { /* ... */ }
executeAll(context: PluginContext): Promise<PluginResult[]> { /* ... */ }
}
// En realite, il n'y a qu'un seul "plugin" : le processeur d'images
// Fix : juste le code dont tu as besoin
class ImageProcessor {
async process(image: Buffer): Promise<ProcessedImage> { /* ... */ }
}
Dead Code
Du code qui n'est jamais exécuté. Fonctions non appelees, variables non lues, branches impossibles. Le dead code ment : il fait croire qu'il sert a quelque chose. Il complique la comprehension.
typescript// Smell : fonction jamais appelee
function legacyFormatDate(date: Date): string {
// 30 lignes de formatage
// personne n'appelle cette fonction depuis 2 ans
return "";
}
// Fix : supprime-le. Git se souvient.
Si tu as peur de le supprimer, git log est ton ami. Le code supprime n'est pas perdu. Il est dans l'historique. Tu trouveras d'autres exemples de détection de smells sur paltemps.fr.
Résumé
- Les code smells sont des signaux, pas des bugs -- le code fonctionne mais sa conception pose problème
- Long Method et God Object sont les plus courants et les plus visibles
- Feature Envy indique que le code est au mauvais endroit
- Primitive Obsession se resout avec des types dédiés en TypeScript
- Data Clumps deviennent des objets coherents
- Le dead code se supprime -- Git se souvient
- Chaque smell a un refactoring associe, détaillé dans le prochain article
Article précédent : 12 - Abstractions prematurees vs tardives
Article suivant : 14 - Techniques de refactoring