Catalogo dei code smell

Catalogo dei code smell

Un code smell è un segnale nel codice: qualcosa che funziona ma che, prima o poi, renderà caro un cambiamento. Questo catalogo raccoglie gli smell descritti da Martin Fowler in Refactoring, divisi nelle cinque famiglie usate da refactoring.guru. Lo userete per tutto il corso: nella caccia del primo giorno, a ogni ticket, e quando dovrete spiegare perché una soluzione è meglio di un'altra.

Come usare il catalogo

Uno smell non è un errore. Il codice può avere uno smell e funzionare benissimo, oggi. Il problema arriva quando qualcuno deve cambiarlo.

Davanti a un pezzo di codice sospetto, il lavoro ha tre passi:

  1. Dare un nome. Trovate nel catalogo lo smell che descrive quello che vedete. Avere un nome serve a parlarne con i colleghi, e anche a chiedere all'AI qualcosa di preciso.
  2. Chiedersi quale cambiamento diventa costoso. Uno smell fa male solo se il codice deve cambiare in quella direzione. Una funzione lunga che nessuno tocca da cinque anni può restare lì.
  3. Decidere se intervenire. A volte la risposta è sì, subito. A volte è "lo segno in DEBITO.md e ci torno quando serve".

Ogni scheda ha quattro parti: i segnali per riconoscerlo, perché fa male, i refactoring che di solito lo risolvono (con i nomi del catalogo di Fowler) e un esempio breve. Quando lo smell viola in modo evidente un principio SOLID, c'è una riga che lo dice.

Smell e principi

Molti smell sono il sintomo visibile di un principio violato. Se vi viene in mente un principio SOLID mentre leggete il codice, cercate lo smell che lo accompagna, e viceversa.

Bloaters

Codice che è cresciuto troppo: metodi, classi o liste di parametri diventati così grandi che nessuno riesce più a tenerli in testa tutti insieme. Di solito non nascono grandi, ci arrivano un pezzo alla volta.

Long Method

Segnali. Una funzione di decine di righe, spesso divisa in "sezioni" da commenti. Per capire cosa fa bisogna leggerla tutta, e per cambiarne un pezzo bisogna capire anche gli altri.

Perché fa male. Ogni modifica rischia di rompere una parte che non c'entra. Le singole parti non si possono provare da sole, e la stessa logica finisce copiata altrove perché estrarla sembra troppo complicato.

Refactoring tipici. Extract Function (una funzione per ogni sezione), Replace Temp with Query, Decompose Conditional.

Principio. SRP: la funzione fa più cose.

Esempio.

function restituisci(prestito: Prestito, oggi: Date): void {
    // calcola il ritardo
    const giorni = differenzaInGiorni(prestito.scadenza, oggi);
    let multa = 0;
    if (giorni > 0) {
        multa = giorni * 0.5;
        if (prestito.socio.studente) multa = multa / 2;
    }
    // aggiorna il catalogo
    prestito.copia.disponibile = true;
    catalogo.salva(prestito.copia);
    // avvisa chi aspettava il libro
    const attesa = prenotazioni.prima(prestito.copia.titolo);
    if (attesa) posta.invia(attesa.email, "Il libro che aspettavi è tornato");
}

Large Class

Segnali. Una classe con molti campi e molti metodi, spesso con un nome generico (Manager, Service, Gestore). Gruppi di campi che si usano solo tra loro, come se dentro ci fossero più classi.

Perché fa male. Chiunque lavori su una qualsiasi funzionalità deve toccare quella classe. I conflitti tra colleghi aumentano, e capire cosa succede richiede di leggere tutto. Il caso estremo è il God Object.

Refactoring tipici. Extract Class, Extract Superclass, Replace Type Code with Subclasses.

Principio. SRP: la classe ha troppi motivi per cambiare.

Esempio.

class Condominio {
    condomini: Condomino[];
    millesimi: Map<string, number>;
    speseOrdinarie: Spesa[];
    speseStraordinarie: Spesa[];
    fornitori: Fornitore[];
    interventiProgrammati: Intervento[];
    verbaliAssemblea: Verbale[];
    prossimaAssemblea: Date | null;

    ripartisciSpese(): Rata[] { /* ... */ }
    convocaAssemblea(data: Date): void { /* ... */ }
    registraIntervento(intervento: Intervento): void { /* ... */ }
    stampaBilancio(): string { /* ... */ }
}

Spese, manutenzione e assemblee sono tre classi che vivono nella stessa.

Primitive Obsession

Segnali. Concetti del dominio rappresentati con tipi primitivi: una stringa per un codice, un numero per un importo, una stringa "S", "M", "L" per una taglia. La stessa validazione compare in più punti.

Perché fa male. Le regole del concetto sono sparse ovunque, e quando cambiano bisogna trovarle tutte. Il compilatore non vi ferma se passate una taglia dove serviva un codice.

Refactoring tipici. Replace Primitive with Object, Replace Type Code with Subclasses, Introduce Parameter Object.

Esempio.

function noleggia(codiceBici: string, taglia: string, telefono: string): void {
    if (!/^[A-Z]{2}\d{4}$/.test(codiceBici)) throw new Error("codice bici non valido");
    if (taglia !== "S" && taglia !== "M" && taglia !== "L") throw new Error("taglia non valida");
    // gli stessi due controlli ci sono anche in restituisci() e in sposta()
}

Un tipo CodiceBici che si valida da solo nel costruttore e un tipo Taglia = "S" | "M" | "L" tolgono i controlli ripetuti.

Long Parameter List

Segnali. Una funzione con molti parametri, spesso dello stesso tipo, e chiamate in cui non si capisce quale valore va dove. Parametri booleani che cambiano il comportamento della funzione.

Perché fa male. È facile scambiare due argomenti senza che nessuno se ne accorga. Ogni parametro nuovo cambia la firma e costringe a toccare tutte le chiamate.

Refactoring tipici. Introduce Parameter Object, Preserve Whole Object, Remove Flag Argument.

Esempio.

function iscrivi(nome: string, cognome: string, email: string, strumento: string, livello: number,
                 giorno: string, ora: string, insegnante: string, conSaggio: boolean, minorenne: boolean): void {
    // ...
}

iscrivi("Sara", "Neri", "sara@example.com", "violino", 2, "martedì", "17:00", "Bassi", true, false);

Leggendo la chiamata, cosa vogliono dire true e false?

Data Clumps

Segnali. Gli stessi tre o quattro dati che viaggiano sempre insieme, come parametri o come campi: via, civico, CAP e città; data di inizio e di fine.

Perché fa male. Il gruppo ha un significato che il codice non dice. Quando cambia una regola (un controllo sul CAP, un campo in più), bisogna rincorrere tutte le firme.

Refactoring tipici. Extract Class, Introduce Parameter Object, Preserve Whole Object.

Esempio.

function registraFornitore(nome: string, via: string, civico: string, cap: string, citta: string): void { /* ... */ }
function inviaRaccomandata(destinatario: string, via: string, civico: string, cap: string, citta: string): void { /* ... */ }
function distanzaDalCondominio(via: string, civico: string, cap: string, citta: string): number { /* ... */ }

Una classe Indirizzo rende esplicito quello che le tre firme ripetono.

Object-Orientation Abusers

Gli strumenti della programmazione a oggetti usati a metà o al contrario: ereditarietà dove non serve, condizioni sul tipo dove servirebbe il polimorfismo, classi che fanno la stessa cosa con nomi diversi.

Switch Statements

Segnali. Uno switch o una catena di if sul tipo di qualcosa, e lo stesso switch ripetuto in più funzioni. Ogni caso nuovo va aggiunto in tutti.

Perché fa male. Aggiungere una variante vuol dire cercare tutti gli switch che la riguardano. Basta dimenticarne uno per avere un bug, e spesso il compilatore non se ne accorge.

Refactoring tipici. Replace Conditional with Polymorphism, Replace Type Code with Subclasses. A volte basta una tabella di dati al posto dello switch.

Principio. OCP: per estendere bisogna modificare.

Esempio.

function durataPrestito(materiale: Materiale): number {
    switch (materiale.tipo) {
        case "libro": return 30;
        case "dvd": return 7;
        case "rivista": return 14;
    }
}

function rinnoviMassimi(materiale: Materiale): number {
    switch (materiale.tipo) {
        case "libro": return 2;
        case "dvd": return 0;
        case "rivista": return 1;
    }
}

Quando arrivano gli audiolibri, quanti switch vanno toccati in tutto il programma?

Temporary Field

Segnali. Campi di una classe che hanno un valore solo durante un certo calcolo e per il resto del tempo restano vuoti o con valori vecchi.

Perché fa male. Chi legge la classe pensa che quei campi facciano parte del suo stato. Se un altro metodo li usa nel momento sbagliato, trova valori senza senso.

Refactoring tipici. Extract Class, Introduce Special Case, oppure variabili locali dentro il metodo.

Esempio.

class SchedaAllenamento {
    esercizi: Esercizio[];
    private caricoTotale = 0;   // ha senso solo dentro volumeMedio()
    private serieContate = 0;   // idem

    volumeMedio(): number {
        this.caricoTotale = 0;
        this.serieContate = 0;
        for (const e of this.esercizi) {
            this.caricoTotale += e.carico * e.ripetizioni * e.serie;
            this.serieContate += e.serie;
        }
        return this.caricoTotale / this.serieContate;
    }
}

Refused Bequest

Segnali. Una sottoclasse eredita metodi che non usa, li lascia vuoti o li fa lanciare un errore.

Perché fa male. Chi usa la classe base si aspetta che tutti i suoi metodi funzionino. Se arriva la sottoclasse sbagliata, il programma si rompe in un punto lontano da dove è nato il problema.

Refactoring tipici. Push Down Method, Replace Subclass with Delegate, Replace Superclass with Delegate.

Principio. LSP: la sottoclasse non si può usare al posto della base.

Esempio.

class Corso {
    assegnaAula(aula: Aula): void { /* ... */ }
    prenotaStrumento(strumento: Strumento): void { /* ... */ }
    fissaSaggio(data: Date): void { /* ... */ }
}

class CorsoDiTeoria extends Corso {
    prenotaStrumento(strumento: Strumento): void {
        throw new Error("il corso di teoria non usa strumenti");
    }
    fissaSaggio(data: Date): void {
        // il corso di teoria non ha saggio: non fa niente
    }
}

Alternative Classes with Different Interfaces

Segnali. Due classi fanno lo stesso lavoro ma con metodi che si chiamano in modo diverso o hanno i parametri in un ordine diverso. Chi le usa deve sapere quale ha davanti.

Perché fa male. Non si può scambiare una con l'altra, e ogni punto che le usa ha il suo if. Una terza classe dello stesso tipo moltiplica gli if.

Refactoring tipici. Change Function Declaration (stessi nomi e stessi parametri), Move Function, Extract Superclass o un'interfaccia comune.

Esempio.

class AvvisoSms {
    mandaMessaggio(numero: string, testo: string): void { /* ... */ }
}

class AvvisoMail {
    inviaEmail(testo: string, indirizzo: string, oggetto: string): void { /* ... */ }
}

if (cliente.preferisceSms) {
    new AvvisoSms().mandaMessaggio(cliente.telefono, testo);
} else {
    new AvvisoMail().inviaEmail(testo, cliente.email, "Noleggio in scadenza");
}

Change Preventers

Smell che rendono i cambiamenti lenti e rischiosi: o una classe cambia per troppi motivi diversi, o un singolo cambiamento si sparpaglia in troppe classi.

Divergent Change

Segnali. La stessa classe va modificata per motivi che non hanno niente a che fare tra loro: oggi per una regola di business, domani per un formato di stampa, dopodomani per un fornitore.

Perché fa male. Persone diverse, con obiettivi diversi, lavorano sullo stesso file. Ogni modifica rischia di rompere qualcosa che appartiene a un'altra area.

Refactoring tipici. Extract Class, Split Phase, Move Function.

Principio. SRP: più di un motivo per cambiare.

Esempio.

class TesseraSocio {
    calcolaScadenza(): Date { /* cambia quando cambiano gli abbonamenti */ }
    stampaBadge(): Uint8Array { /* cambia quando cambia la stampante dei badge */ }
    esportaRigaContabile(): string { /* cambia quando cambia il formato del commercialista */ }
}

Shotgun Surgery

Segnali. Un cambiamento che sembra piccolo costringe a toccare tante classi o tanti file, un pezzetto per ciascuno. È il contrario di Divergent Change.

Perché fa male. È facile dimenticare un punto, e il programma resta incoerente. Ogni cambiamento richiede di ricordarsi la mappa di tutti i posti da toccare.

Refactoring tipici. Move Function, Move Field, Combine Functions into Class, Inline Class.

Esempio. Per aggiungere un nuovo tipo di materiale in biblioteca bisogna toccare cinque file:

// catalogo.ts
const TIPI = ["libro", "dvd", "rivista"];
// prestiti.ts
const durata = tipo === "dvd" ? 7 : tipo === "rivista" ? 14 : 30;
// multe.ts
const tariffa = tipo === "dvd" ? 1 : 0.5;
// etichette.ts
const colore = { libro: "blu", dvd: "rosso", rivista: "verde" }[tipo];
// report.ts
const sezioni = ["Libri", "DVD", "Riviste"];

Parallel Inheritance Hierarchies

Segnali. Due gerarchie di classi che crescono in coppia: ogni volta che si aggiunge una sottoclasse in una, bisogna aggiungerne una corrispondente nell'altra.

Perché fa male. Il lavoro raddoppia a ogni variante, e prima o poi qualcuno aggiunge una metà e dimentica l'altra.

Refactoring tipici. Move Function e Move Field per far confluire una gerarchia nell'altra.

Esempio.

abstract class Strumento { /* ... */ }
class Violino extends Strumento { /* ... */ }
class Chitarra extends Strumento { /* ... */ }

abstract class Manutenzione { /* ... */ }
class ManutenzioneViolino extends Manutenzione { /* ... */ }
class ManutenzioneChitarra extends Manutenzione { /* ... */ }

// un'arpa nuova vuol dire Arpa e anche ManutenzioneArpa

Dispensables

Cose che si potrebbero togliere, e il codice diventerebbe più chiaro: duplicati, classi che non fanno quasi niente, codice morto, generalità che nessuno usa, commenti che spiegano quello che il codice non riesce a dire.

Comments

Segnali. Un commento lungo che spiega cosa fa un pezzo di codice difficile da leggere. Il commento è un deodorante: copre l'odore invece di toglierlo.

Perché fa male. Il codice cambia e il commento no, e dopo un po' dice il falso. Spesso basterebbe un nome migliore o una funzione estratta.

Refactoring tipici. Extract Function, Rename, Introduce Assertion.

Esempio.

// può prenotare la sala se è maggiorenne, non è sospeso, ha l'abbonamento valido
// e non ha già due prenotazioni questa settimana
if (s.eta >= 18 && !s.sospeso && s.scadenza >= oggi && prenotazioniSettimana(s) < 2) {
    prenota(s, sala);
}

Con una funzione puoPrenotare(socio, oggi) il commento non serve più. I commenti che spiegano perché si è fatta una scelta invece restano utili.

Duplicate Code

Segnali. Lo stesso pezzo di codice, o quasi lo stesso, in più punti. Spesso è nato da un copia e incolla con una piccola modifica.

Perché fa male. Una correzione va fatta in tutte le copie, e di solito ne resta indietro una. Le copie poi divergono, e nessuno sa più quale sia quella giusta.

Refactoring tipici. Extract Function, Pull Up Method, Slide Statements, Form Template Method.

Esempio.

function rataRiscaldamento(c: Condomino, totale: number): number {
    const quota = millesimi.get(c.id) ?? 0;
    return Math.round(totale * quota / 1000 * 100) / 100;
}

function rataAscensore(c: Condomino, totale: number): number {
    const quota = millesimiAscensore.get(c.id) ?? 0;
    return Math.round(totale * quota / 1000 * 100) / 100;
}

Lazy Class

Segnali. Una classe che fa così poco da non giustificare la propria esistenza: un solo metodo di una riga, usato in un solo punto.

Perché fa male. Ogni classe è un nome in più da conoscere e un file in più da aprire. Se non porta niente, è solo rumore.

Refactoring tipici. Inline Class, Collapse Hierarchy.

Esempio.

class FormattatoreData {
    formatta(data: Date): string {
        return data.toLocaleDateString("it-IT");
    }
}

// usata una volta sola, in stampaRicevuta()

Data Class

Segnali. Una classe che contiene solo campi, mentre tutta la logica che li riguarda vive altrove, in un "service" che li legge e li modifica.

Perché fa male. Le regole su quei dati si sparpagliano nelle classi che li usano e finiscono duplicate. La classe non protegge niente: chiunque può metterla in uno stato senza senso.

Refactoring tipici. Move Function (portare la logica vicino ai dati), Encapsulate Record, Remove Setting Method.

Esempio.

class Prestito {
    socio: Socio;
    copia: Copia;
    scadenza: Date;
    restituito: boolean;
}

class PrestitiService {
    inRitardo(p: Prestito, oggi: Date): boolean {
        return !p.restituito && p.scadenza < oggi;
    }
}

inRitardo appartiene a Prestito.

Dead Code

Segnali. Funzioni che nessuno chiama, variabili mai lette, rami di un if che non si eseguono mai, blocchi commentati "nel caso servissero".

Perché fa male. Chi legge deve capire anche il codice morto, e spesso non sa che è morto. Viene aggiornato a ogni refactoring senza motivo, e qualche volta qualcuno lo riattiva per sbaglio.

Refactoring tipici. Remove Dead Code. La storia di git ricorda tutto, non serve tenerlo commentato.

Esempio.

const NUOVO_CALCOLO_MULTE = false;

function multa(giorni: number): number {
    if (NUOVO_CALCOLO_MULTE) {
        return giorni * 0.3 + 1;   // non viene mai eseguito
    }
    return giorni * 0.5;
}

// function multaVecchia(giorni: number): number {
//     return giorni * 0.25;
// }

Speculative Generality

Segnali. Interfacce con una sola implementazione, parametri che nessuno passa, classi astratte con un solo figlio, "punti di estensione" preparati per un futuro che non è mai arrivato.

Perché fa male. Il codice diventa più lungo e più difficile da seguire per gestire casi che non esistono. Quando il futuro arriva davvero, quasi sempre non assomiglia a quello che si era previsto.

Refactoring tipici. Collapse Hierarchy, Inline Function, Inline Class, Change Function Declaration (togliere i parametri inutili).

Esempio.

interface FornitoreDiMezzi {
    disponibili(zona: string, opzioni?: Record<string, unknown>): Mezzo[];
}

class FornitoreDiMezziFactory {
    crea(tipo: string): FornitoreDiMezzi {
        return new FornitoreBiciInterno();   // esiste solo questo, da tre anni
    }
}

Couplers

Classi troppo legate tra loro: si scambiano dettagli interni, si chiamano a catena, o una fa solo da passacarte per l'altra. Cambiarne una vuol dire cambiare anche le altre.

Feature Envy

Segnali. Un metodo che usa i dati di un altro oggetto più dei dati della propria classe.

Perché fa male. La logica sta lontano dai dati che usa. Quando i dati cambiano, la modifica tocca una classe che non sembrava coinvolta.

Refactoring tipici. Move Function, Extract Function.

Esempio.

class ReportPrestiti {
    giorniDiRitardo(p: Prestito, oggi: Date): number {
        if (p.restituito) return 0;
        const fine = p.dataRestituzione ?? oggi;
        return Math.max(0, differenzaInGiorni(p.scadenza, fine));
    }
}

Il metodo parla solo di Prestito: il suo posto è prestito.giorniDiRitardo(oggi).

Inappropriate Intimacy

Segnali. Due classi che leggono e modificano l'una i campi interni dell'altra, spesso in tutte e due le direzioni.

Perché fa male. Non si può cambiare una senza cambiare l'altra. Le regole di ciascuna (un ordinamento, un limite) vengono violate da chi le tocca dall'esterno.

Refactoring tipici. Move Function, Move Field, Hide Delegate, Change Bidirectional Association to Unidirectional.

Esempio.

class Corso {
    allievi: Allievo[] = [];
}

class Allievo {
    corsi: Corso[] = [];

    iscriviA(corso: Corso): void {
        this.corsi.push(corso);
        corso.allievi.push(this);   // modifica l'interno di Corso
        corso.allievi.sort((a, b) => a.cognome.localeCompare(b.cognome));
    }
}

Message Chains

Segnali. Una catena di chiamate per arrivare al dato che serve: a.b().c().d().

Perché fa male. Chi scrive la catena dipende dalla struttura di tutti gli oggetti attraversati. Se uno di loro cambia, si rompono tutte le catene che ci passano. È la violazione della Legge di Demetra ("parla solo con i tuoi vicini").

Refactoring tipici. Hide Delegate, Extract Function, Move Function.

Esempio.

const cap = condominio.getAmministratore().getStudio().getIndirizzo().getCap();

Middle Man

Segnali. Una classe che non fa altro che passare le chiamate a un'altra, senza aggiungere niente.

Perché fa male. È un passaggio in più da leggere e da mantenere, e ogni metodo nuovo della classe vera va ricopiato anche lì. Attenzione però: una classe intermedia che semplifica un sottosistema complicato, o che lo nasconde a chi lo usa, ha un motivo per esistere. Il problema è quando non aggiunge niente.

Refactoring tipici. Remove Middle Man, Inline Function.

Esempio.

class Segreteria {
    constructor(private readonly registro: Registro) {}

    iscrivi(allievo: Allievo, corso: Corso): void { this.registro.iscrivi(allievo, corso); }
    ritira(allievo: Allievo, corso: Corso): void { this.registro.ritira(allievo, corso); }
    elenco(corso: Corso): Allievo[] { return this.registro.elenco(corso); }
}

Incomplete Library Class

Segnali. Una libreria esterna non fa una cosa che vi serve, e le funzioni che la completano sono sparse e ricopiate in giro per il progetto.

Perché fa male. La libreria non si può modificare, e il codice che la completa non ha un posto suo. Quando cambia la libreria, bisogna cercare tutte le aggiunte.

Refactoring tipici. Introduce Foreign Method, Introduce Local Extension (una classe vostra che usa la libreria e aggiunge quello che manca).

Esempio.

// la libreria delle date non sa contare i giorni lavorativi:
// questa funzione è stata riscritta, con piccole differenze, in tre moduli
function giorniLavorativi(da: Date, a: Date): number {
    let n = 0;
    for (let d = new Date(da); d <= a; d.setDate(d.getDate() + 1)) {
        if (d.getDay() !== 0 && d.getDay() !== 6) n++;
    }
    return n;
}

Smell del codice generato

Gli assistenti AI scrivono codice plausibile molto in fretta, e con il codice arrivano anche alcuni smell ricorrenti. Riconoscerli fa parte della revisione.

  • Duplicazione plausibile. Due funzioni quasi identiche generate in momenti diversi, perché l'assistente non sapeva che la prima esisteva già. È Duplicate Code, ma più difficile da notare perché i nomi cambiano.
  • Astrazioni senza motivo. Interfacce con una sola implementazione, factory che creano un solo tipo, classi base "per il futuro". È Speculative Generality.
  • Test modificati per farli passare. Invece di correggere il codice, l'assistente rende il test più permissivo:
// il test di prima
expect(ordine.totale).toBe(78.29);

// il test dopo la "correzione"
expect(ordine.totale).toBeGreaterThan(0);
  • Modifiche fuori scope. Per risolvere un ticket l'assistente riordina, rinomina e "migliora" file che il ticket non toccava. Il diff diventa enorme, ed è difficile vedere cosa è cambiato davvero.
  • Commenti che parafrasano il codice. Un commento per ogni riga che ripete quello che la riga fa già. Sono Comments nella versione peggiore: non spiegano niente e invecchiano subito.

Riferimenti