Refactoring a piccoli passi

Refactoring a piccoli passi

Durante il corso cambierete molto codice che funziona già: lo renderete più facile da modificare senza cambiare quello che fa. Questa nota spiega come farlo senza rompere niente.

Cos'è il refactoring

Martin Fowler lo definisce così: una modifica alla struttura interna del software che lo rende più facile da capire e da cambiare, senza cambiarne il comportamento osservabile. Chi usa il programma non deve accorgersi di niente.

Ne segue una regola pratica: refactoring e nuove funzionalità non si fanno nello stesso passo. Fowler usa l'immagine dei due cappelli. Con il cappello del refactoring cambiate la forma del codice e non aggiungete niente; con il cappello delle funzionalità aggiungete comportamento e non riorganizzate. Si può cambiare cappello anche ogni dieci minuti, ma uno alla volta.

Perché due cappelli

Se un test diventa rosso mentre state facendo tutte e due le cose insieme, non sapete se avete rotto qualcosa o se il comportamento nuovo è sbagliato. Separandole, ogni rosso ha una causa sola.

La rete di sicurezza

Un refactoring senza test è una scommessa: speri di non aver cambiato il comportamento, ma non lo sai. I test che descrivono quello che il codice fa oggi sono la rete: con quella sotto, la struttura si cambia con tranquillità.

In TicketHub la suite e2e copre la lista degli eventi, il checkout e l'emissione dei biglietti. Il resto non ha test. Prima di rifattorizzare una parte scoperta, la rete va costruita.

Test di caratterizzazione

Michael Feathers, in Working Effectively with Legacy Code, chiama legacy il codice senza test, e propone un modo per metterlo in sicurezza: il test di caratterizzazione. Non descrive come il codice dovrebbe comportarsi, ma come si comporta adesso, anche quando sbaglia.

Si procede così: si chiama il codice con alcuni input, si guarda cosa restituisce, e si scrive quel valore come risultato atteso. Ecco una vecchia funzione che calcola la multa per i ritardi in biblioteca:

function multa(giorniDiRitardo: number, studente: boolean): number {
    let importo = giorniDiRitardo * 0.5;
    if (studente) importo = importo / 2;
    return importo;
}

E i test che ne fotografano il comportamento:

describe("multa, comportamento attuale", () => {
    it("nessun ritardo, nessuna multa", () => {
        expect(multa(0, false)).toBe(0);
    });
    it("dieci giorni di ritardo", () => {
        expect(multa(10, false)).toBe(5);
    });
    it("dieci giorni di ritardo per uno studente", () => {
        expect(multa(10, true)).toBe(2.5);
    });
    it("ritardo negativo: oggi restituisce una multa negativa", () => {
        // comportamento strano, fissato così com'è e annotato in DEBITO.md
        expect(multa(-3, false)).toBe(-1.5);
    });
});

L'ultimo test fissa un comportamento sbagliato: una multa negativa. Non lo correggete adesso, perché state rifattorizzando e il comportamento non deve cambiare. Lo annotate e lo correggerete in un passo separato, con il cappello delle funzionalità.

Da verde a verde

Il ritmo di un refactoring è sempre lo stesso:

  1. si parte con tutti i test verdi;
  2. si fa un passo piccolo, uno solo;
  3. si rilanciano i test;
  4. se sono verdi si va avanti, se sono rossi si annulla il passo e lo si rifà più piccolo.

Annullare invece di cercare il guasto sembra uno spreco, ma il passo era piccolo e rifarlo costa pochi minuti. Cercare il guasto in un passo grande può costare un pomeriggio.

I refactoring di base

Fowler ne cataloga decine. Questi sono quelli che userete di più, e ognuno risponde a uno o più smell del Catalogo dei code smell.

Extract Function

Quando serve. Long Method, Duplicate Code, Comments: un pezzo di codice ha un'intenzione precisa che si potrebbe nominare.

Esempio.

// prima
function restituisci(prestito: Prestito, oggi: Date): void {
    const giorni = differenzaInGiorni(prestito.scadenza, oggi);
    let importo = 0;
    if (giorni > 0) {
        importo = giorni * 0.5;
        if (prestito.socio.studente) importo = importo / 2;
    }
    prestito.multa = importo;
    prestito.copia.disponibile = true;
}

// dopo
function restituisci(prestito: Prestito, oggi: Date): void {
    prestito.multa = calcolaMulta(prestito, oggi);
    prestito.copia.disponibile = true;
}

function calcolaMulta(prestito: Prestito, oggi: Date): number {
    const giorni = differenzaInGiorni(prestito.scadenza, oggi);
    if (giorni <= 0) return 0;
    const importo = giorni * 0.5;
    return prestito.socio.studente ? importo / 2 : importo;
}

Rischio principale. Le variabili usate sia dentro sia fuori dal pezzo estratto: vanno passate come parametri o restituite, e se qualcuna viene modificata il comportamento può cambiare.

Rename

Quando serve. Un nome che non dice cosa fa la cosa, o che dice una cosa diversa. È il refactoring più semplice e quello che rende di più.

Esempio.

// prima
function calc(p: Prestito, d: Date): number { /* ... */ }

// dopo
function giorniDiRitardo(prestito: Prestito, oggi: Date): number { /* ... */ }

Rischio principale. Dimenticare qualche uso, soprattutto in stringhe, file di configurazione o test. Usate la funzione di rinomina dell'editor, non la ricerca e sostituzione.

Move Function

Quando serve. Feature Envy, Shotgun Surgery: una funzione usa più i dati di un'altra classe che quelli della propria.

Esempio.

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

// dopo
class Prestito {
    inRitardo(oggi: Date): boolean {
        return !this.restituito && this.scadenza < oggi;
    }
}

Rischio principale. I chiamanti: vanno aggiornati tutti. Spesso conviene lasciare per un passo la vecchia funzione che chiama la nuova, e toglierla nel passo successivo.

Extract Class

Quando serve. Large Class, Data Clumps, Divergent Change: una parte della classe ha una vita sua.

Esempio.

// prima
class Socio {
    nome: string;
    via: string;
    civico: string;
    cap: string;
    citta: string;
    etichettaPostale(): string {
        return this.nome + "\n" + this.via + " " + this.civico + "\n" + this.cap + " " + this.citta;
    }
}

// dopo
class Indirizzo {
    constructor(
        readonly via: string,
        readonly civico: string,
        readonly cap: string,
        readonly citta: string,
    ) {}
    perEtichetta(): string {
        return this.via + " " + this.civico + "\n" + this.cap + " " + this.citta;
    }
}

class Socio {
    constructor(readonly nome: string, readonly indirizzo: Indirizzo) {}
    etichettaPostale(): string {
        return this.nome + "\n" + this.indirizzo.perEtichetta();
    }
}

Rischio principale. Estrarre troppo: una classe nuova deve avere un nome chiaro e una responsabilità sua, altrimenti è una Lazy Class.

Inline

Quando serve. Lazy Class, Middle Man, Speculative Generality: una funzione o una classe il cui corpo è chiaro quanto il suo nome, oppure un'indirezione che non serve più. È il contrario di Extract.

Esempio.

// prima
function haBiciDisponibili(numero: number): boolean {
    return numero > 0;
}

if (haBiciDisponibili(stallo.biciPresenti)) { /* ... */ }

// dopo
if (stallo.biciPresenti > 0) { /* ... */ }

Rischio principale. Togliere un nome che spiegava qualcosa. Se senza la funzione il codice diventa meno chiaro, lasciatela.

Replace Magic Number with Symbolic Constant

Quando serve. Numeri e stringhe scritti nel codice senza spiegazione, soprattutto se compaiono in più punti.

Esempio.

// prima
if (Date.now() - socio.ultimoAccesso > 2592000000) {
    sospendi(socio, 0.5);
}

// dopo
const TRENTA_GIORNI_IN_MS = 30 * 24 * 60 * 60 * 1000;
const RIDUZIONE_SOSPENSIONE = 0.5;

if (Date.now() - socio.ultimoAccesso > TRENTA_GIORNI_IN_MS) {
    sospendi(socio, RIDUZIONE_SOSPENSIONE);
}

Rischio principale. Raggruppare sotto la stessa costante due numeri uguali per caso. Se lo 0.5 della multa e lo 0.5 della sospensione hanno motivi diversi, sono due costanti.

Un passo, un commit

Ogni passo che lascia i test verdi merita un commit, con un messaggio che dice cosa è cambiato:

git commit -m "Estrae calcolaMulta da restituisci"
git commit -m "Sposta inRitardo in Prestito"

Se il passo successivo va storto, tornate all'ultimo commit verde e ripartite da lì. È lo stesso schema che vedrete su develop: la soluzione costruita in classe arriva come una serie di commit, uno per ogni passo, e il git log racconta come ci si è arrivati.

Quando non rifattorizzare

Il refactoring si fa quando serve al prossimo cambiamento, non per principio. Ci sono casi in cui conviene lasciar stare:

  • codice che sta per essere buttato o riscritto;
  • codice che funziona e che nessuno deve toccare;
  • quando non c'è tempo di costruire la rete di test: meglio una modifica piccola e documentata in DEBITO.md che un refactoring alla cieca;
  • a ridosso di una consegna, se il refactoring non è necessario per la consegna stessa.

Riferimenti

  • Martin Fowler, Refactoring: Improving the Design of Existing Code, seconda edizione, 2018. Il catalogo è anche online: https://refactoring.com/catalog
  • Michael Feathers, Working Effectively with Legacy Code, 2004.