Matteo Pipitone

La storia di un refactor: il buttafuori e la catena dei controlli

18 min di lettura
  • software
  • clean-code

Il problema, raccontato con una metafora

C’è un locale con un buttafuori all’ingresso. Una persona vuole entrare. Il buttafuori la fa entrare solo se una serie di controlli è soddisfatta. E i controlli non sono tutti uguali:

  1. immediato: la persona è sulla lista degli invitati? (non serve nulla di esterno)
  2. con collaboratore: il suo documento risulta valido all’anagrafe? (serve interrogare un servizio esterno)
  3. contestuale: rispetta il dress code previsto per la serata di stasera? (dipende da un contesto)
  4. di sottocaso: se è un VIP, valgono regole tutte sue (una famiglia separata di controlli)

Questa metafora cattura la struttura del problema reale: un soggetto da validare (la persona ≈ ciò che vogliamo processare); regole eterogenee, alcune immediate, altre che dipendono da collaboratori esterni o dal contesto; un sottocaso con regole proprie (il VIP); un esito binario, si entra o non si entra.

La domanda di fondo del refactor è sempre la stessa: dove mettiamo le regole, e come se ne aggiungono di nuove senza dover riaprire e modificare il codice esistente? Il caso del biglietto, “e se domani il locale inizia a chiedere anche il biglietto?”, sarà il nostro banco di prova ricorrente.

Soluzione 1: il buttafuori che controlla tutto a memoria

Il primo abbozzo è il più ingenuo e il più onesto: il buttafuori fa tutto da sé, in linea, tenendo le regole “a memoria”. Per giunta è la persona a porgergli già in mano ciò che serve (presenta il documento di sua iniziativa). Il controllo è una sequenza di “se… allora respingi”.

let_in(guest, document):
    if the guest is not on the guest list -> reject
    if the document is not valid -> reject
    let the guest in

Lo stesso schizzo in Java:

class Bouncer {
  private final GuestList guestList;

  Bouncer(GuestList guestList) {
    this.guestList = guestList;
  }

  void letIn(Guest guest, Document document) {
    if (!guestList.contains(guest)) {
      return;
    }
    if (!document.isValid()) {
      return;
    }
    // ... the guest enters
  }
}

Pro

  • È la cosa più semplice che possa funzionare: l’incarnazione pura del KISS.
  • Si legge tutta in un colpo d’occhio, dall’alto verso il basso. Zero indirezioni, zero astrazioni.
  • Per una sola serata e poche regole stabili, sarebbe già più che accettabile.

Contro

  • Viola l’Open/Closed. Aggiungere il controllo del biglietto significa riaprire il metodo del buttafuori e infilarci un altro “se”. Il codice è “aperto alle modifiche”: esattamente il contrario di ciò che voglio.
  • Il metodo cresce in modo lineare con i controlli: prima o poi diventa un lenzuolo di if.
  • La persona è costretta a procurarsi e porgere il documento: una responsabilità che non le compete, e che fa trapelare un dettaglio (“per validare serve il documento”) verso l’esterno.
  • I valori sono cablati nel metodo: le condizioni vivono come costanti in mezzo alla logica.

Lezione: il punto di partenza è semplicissimo ma rigido. Funziona, ma ogni cambiamento costa una modifica al cuore della logica.

Soluzione 2: il buttafuori si procura da sé il dato esterno

Primo spostamento di responsabilità: la persona non deve più porgere il documento. Il buttafuori, a partire da un riferimento (un id), va a recuperarlo da sé interrogando l’anagrafe, e poi applica la regola.

let_in(guest):
    if the guest is not on the guest list -> reject
    document = registry.lookup(guest.documentId)
    if the document is not valid -> reject
    let the guest in

In Java, il collaboratore è creato a mano dentro il buttafuori:

class Bouncer {
  private final GuestList guestList;
  private final Registry registry = new Registry(); // tight coupling, not replaceable

  Bouncer(GuestList guestList) {
    this.guestList = guestList;
  }

  void letIn(Guest guest) {
    if (!guestList.contains(guest)) {
      return;
    }
    Document document = registry.lookup(guest.documentId());
    if (!document.isValid()) {
      return;
    }
    // ... the guest enters
  }
}

Pro

  • L’interfaccia pubblica si semplifica e si chiude: chi chiama dice solo “fai entrare questa persona”. Il “come” recuperare i dati di supporto diventa un fatto interno del buttafuori (migliore incapsulamento).
  • La persona non conosce più il documento o l’anagrafe: una dipendenza in meno che esce dai confini.

Contro

  • Il buttafuori ora dipende da un collaboratore concreto che si è creato da solo (si è “scelto” l’anagrafe): accoppiamento forte, difficile da sostituire o da testare in isolamento.
  • L’Open/Closed è ancora violato: i controlli restano if in linea. Aggiungerne uno vuol dire ancora modificare il metodo.
  • Un solo metodo accumula troppe responsabilità: orchestrazione del flusso, recupero dati e logica delle regole, tutto mescolato.
  • Testare una regola in fondo al flusso è caro: per arrivarci devi prima superare i controlli che la precedono e mockare tutti i collaboratori coinvolti, costruendo un contesto complesso solo per verificare un singolo esito.

Lezione: abbiamo migliorato l’incapsulamento (chi chiama sa meno cose), ma non l’estendibilità. Spostare da dove arrivano i dati non basta: il vero nodo è come si compongono le regole.

Soluzione 3: istruire il buttafuori con delle funzioni

Tentativo di guadagnare flessibilità senza ancora introdurre una gerarchia di classi: si inietta il comportamento passando al buttafuori delle funzioni: una funzione per recuperare il documento e un predicato per controllarlo.

let_in(guest, functionThatFetchesTheDocument, predicateThatChecksTheDocument):
    if the guest is not on the guest list -> reject
    document = functionThatFetchesTheDocument(guest.documentId)
    if the document is not valid -> reject
    if the predicate does not pass -> reject
    let the guest in

In Java, le funzioni arrivano come parametri del metodo:

class Bouncer {
  private final GuestList guestList;

  Bouncer(GuestList guestList) {
    this.guestList = guestList;
  }

  void letIn(Guest guest, Function<String, Document> fetchDocument,
             Predicate<Document> checkDocument) {
    if (!guestList.contains(guest)) {
      return;
    }
    Document document = fetchDocument.apply(guest.documentId());
    if (!document.isValid()) {           // hard-wired check...
      return;
    }
    if (!checkDocument.test(document)) { // ...and external check: two places for the same thing
      return;
    }
    // ... the guest enters
  }
}

Pro

  • Un primo passo concettuale verso l’Open/Closed: posso cambiare il comportamento passando funzioni diverse, in certi casi senza toccare il metodo.
  • Più flessibile dello step precedente, e senza il peso di nuove classi o interfacce.

Contro

  • La firma del metodo si gonfia e diventa opaca: passare funzioni a ogni chiamata è scomodo da leggere e da usare, va contro il KISS proprio mentre cerca di essere “elegante”.
  • È una mezza misura confusa: convivono ancora un if cablato e un predicato esterno. Due posti che fanno controlli, responsabilità sovrapposte, e il dubbio su dove stia davvero la regola.
  • Le regole si disperdono sui punti di chiamata (le funzioni nascono dove si invoca): non esiste un’unità di controllo con un nome, isolata e testabile. La composizione non è un concetto di prima classe.

Lezione: la flessibilità “a colpi di funzioni volanti” sembra potente ma sposta la complessità sul chiamante e frammenta le regole. Serviva qualcosa di nominabile, isolato e componibile.

Soluzione 4: ogni controllo diventa un oggetto, la catena dei controlli

Salto di qualità: ogni controllo diventa un oggetto a sé che implementa una piccola interfaccia comune (admits(guest) -> true/false). Il buttafuori riceve una lista di controlli e si limita a chiedere a tutti: “siete tutti d’accordo?”.

Bouncer(listOfChecks)

let_in(guest):
    if NOT all checks admit the guest -> reject
    let the guest in

In Java, ogni controllo è un oggetto con un nome dietro un’interfaccia comune:

interface Check {
  boolean admits(Guest guest);
}

class Bouncer {
  private final List<Check> checks;

  Bouncer(List<Check> checks) {
    this.checks = checks;
  }

  void letIn(Guest guest) {
    if (!checks.stream().allMatch(check -> check.admits(guest))) {
      return;
    }
    // ... the guest enters
  }
}

// one check = one responsibility, testable on its own
class DocumentCheck implements Check {
  private final Registry registry;

  DocumentCheck(Registry registry) {
    this.registry = registry;
  }

  @Override
  public boolean admits(Guest guest) {
    return registry.lookup(guest.documentId()).isValid();
  }
}

// tomorrow, without touching the Bouncer:
class TicketCheck implements Check {
  @Override
  public boolean admits(Guest guest) {
    return guest.hasTicket();
  }
}

E i controlli ora sono entità con un nome: GuestListCheck, DocumentCheck, DressCodeCheck, e domani, semplicemente, TicketCheck.

Pro

  • Open/Closed centrato: aggiungere il controllo del biglietto significa scrivere un nuovo oggetto-controllo e inserirlo nella lista. Il buttafuori non si tocca più: chiuso alla modifica, aperto all’estensione.
  • Singola responsabilità per ogni controllo: ognuno fa una cosa sola, si legge e si testa da solo.
  • Il metodo d’ingresso torna minuscolo e stabile: una riga di orchestrazione.

Contro

  • L’orchestrazione vive ancora dentro il buttafuori: è lui a iterare e a decidere la semantica del “tutti d’accordo”. Il buttafuori è insieme “addetto a questa porta” e “coordinatore dei controlli”: due ruoli in uno.
  • È il buttafuori a dover conoscere e custodire la propria lista di controlli.

Lezione: questo è il punto di svolta. Con un’interfaccia minima e oggetti-controllo ottengo l’estendibilità che cercavo, senza complicare il punto d’uso. Per molti domini, questo è già il punto di equilibrio giusto tra OCP e KISS.

Soluzione 5: astrarre l’intera politica d’ingresso

Ultimo affinamento: il buttafuori non conosce più la catena. Dipende da un’unica astrazione, una politica d’ingresso, e le delega la decisione. È quella politica, internamente, a comporre i singoli controlli, che continuano a ricevere da fuori i propri collaboratori come nella Soluzione 4: il controllo del documento riceve l’anagrafe dal costruttore.

Bouncer(admissionPolicy)

let_in(guest):
    ask the admissionPolicy whether the guest may enter
    let the guest in

admissionPolicy (chained implementation):
    allows(guest) = all checks admit the guest

documentCheck(injected registry):
    admits(guest) = the document fetched from the registry is valid

In Java, il buttafuori conosce solo l’astrazione; la catena vive dentro la politica:

interface AdmissionPolicy {
  boolean allows(Guest guest);
}

class Bouncer {
  private final AdmissionPolicy policy;

  Bouncer(AdmissionPolicy policy) {
    this.policy = policy;
  }

  void letIn(Guest guest) {
    if (!policy.allows(guest)) {
      return;
    }
    // ... the guest enters
  }
}

// the chain is now an internal detail of the policy
class ChainedAdmissionPolicy implements AdmissionPolicy {
  private final List<Check> checks;

  ChainedAdmissionPolicy(List<Check> checks) {
    this.checks = checks;
  }

  @Override
  public boolean allows(Guest guest) {
    return checks.stream().allMatch(check -> check.admits(guest));
  }
}

// ...and whole, interchangeable policies
class OpenDoorPolicy implements AdmissionPolicy {
  @Override
  public boolean allows(Guest guest) {
    return true;
  }
}

class ClosedVenuePolicy implements AdmissionPolicy {
  @Override
  public boolean allows(Guest guest) {
    return false;
  }
}

In più, di quella “politica d’ingresso” possono esistere implementazioni intere e intercambiabili: una politica “sempre sì” per una serata a porte aperte, una politica “sempre no” a locale chiuso, la politica a catena reale per la serata normale. Si può sostituire l’intera politica, non solo i singoli controlli.

Pro

  • Massima separazione delle responsabilità: il buttafuori (la “porta”) è completamente disaccoppiato dalla validazione. L’estendibilità agisce a due livelli: aggiungere un controllo, oppure sostituire in blocco la politica.
  • È il design che scala meglio: la catena è un dettaglio interno, la porta non se ne accorge.

Contro

  • Più pezzi in gioco: due interfacce (la politica d’ingresso e i singoli controlli) più le relative implementazioni. C’è più cerimonia.
  • Il rischio concreto è l’over-engineering: se il dominio è piccolo, questa astrazione a due livelli può essere più di quanto serva: ed è qui che la tensione con il KISS si fa sentire di più.

Lezione: è il design più potente e più “pulito” sulla carta, ma va meritato. Il secondo livello di astrazione (poter sostituire l’intera politica) si giustifica solo quando serve davvero. Altrimenti la Soluzione 4 è già sufficiente e più semplice.

E il VIP? Il sottocaso che merita una porta a parte

Resta il quarto tipo di controllo: il VIP, con regole tutte sue. La tentazione è infilarlo nella catena generica, magari aggiungendo alla persona dei campi “da VIP” che gli altri controlli ignorano. È la strada sbagliata: significherebbe trascinare in ogni controllo parametri che non lo riguardano.

La scelta in spirito Open/Closed è tenere il VIP dietro la sua astrazione dedicata, un validatore VIP separato con i suoi eventuali collaboratori, e sceglierlo a monte. Attenzione: qui non serve una catena come quella ordinaria; per il VIP basta un singolo controllo, invocato direttamente. Famiglie diverse di ospiti evolvono in modo indipendente: cambiare le regole dei VIP non tocca la catena ordinaria, e viceversa.

In Java, il VIP ha la sua astrazione dedicata e la scelta avviene a monte:

// a separate family with its own interface: it does not inherit the standard chain's parameters
interface VipAdmissionPolicy {
  boolean allows(VipGuest vip);
}

// the policy is chosen upstream; the two families evolve independently
boolean admitted = guest.isVip() ? vipPolicy.allows(guest.asVip()) : standardPolicy.allows(guest);

Onestà sul trade-off. Tenere il VIP a parte non è una scelta gratuita, ed è giusto dirlo: due astrazioni parallele per “validare → sì/no” (la catena ordinaria e il validatore VIP) significano che chi legge deve imparare due contratti, e la scelta tra famiglie vive a monte come un ramo per-categoria. È il prezzo pagato per non trascinare campi null (“da VIP”) in tutti i controlli della catena unica: con due famiglie è un buon affare, con molte famiglie diventerebbe un costo. E il validatore VIP è oggi un’astrazione con un solo implementatore e un solo punto di chiamata: l’interfaccia si giustifica se vuoi un contratto esplicito e un seam di test, altrimenti, in pieno KISS, la classe concreta basterebbe. Va tenuta per scelta, non per automatismo.

In breve: separare il VIP è giusto (niente parametri morti, famiglie indipendenti), ma baratta un po’ di uniformità concettuale e introduce un’astrazione che va meritata.

Una terza via: generalizzare il contratto (il voter di Symfony)

C’è un’alternativa che non tocca i dati (niente eredità, niente campi inutili) ma allarga il contratto: rendere il metodo generico, allows(Object subject), così la stessa interfaccia accetta qualsiasi soggetto: è esattamente ciò che fa il VoterInterface di Symfony col suo subject di tipo mixed. E la cosa elegante è che il meccanismo ce l’abbiamo già: il supports() del voter, che si astiene se il soggetto non lo riguarda, è il nostro isInMyScope(...). Il VIP diventerebbe così un voter come gli altri nella stessa catena, e sparirebbe la seconda interfaccia.

// un solo contratto generalizzato: il soggetto è "qualsiasi cosa", il voter si astiene se non lo riguarda
interface AdmissionVoter {
  boolean supports(Object subject); // = isInMyScope
  boolean allows(Object subject);
}

// la regola VIP diventa solo un altro voter nella stessa catena
class VipVoter implements AdmissionVoter {
  @Override
  public boolean supports(Object subject) {
    return subject instanceof VipGuest; // vota solo sugli ospiti VIP
  }

  @Override
  public boolean allows(Object subject) {
    VipGuest vip = (VipGuest) subject;   // ...ma deve fare il downcast
    return vip.isOnVipList();
  }
}

Pro

  • Una sola astrazione invece di due parallele: sparisce la duplicazione concettuale e il ramo “if VIP” a monte.
  • OCP pieno anche sull’aggiunta di famiglie: una famiglia nuova è un voter nuovo, l’orchestratore non si tocca.
  • Nessuna eredità finta, nessun campo null.

Contro

  • Perdi la type safety: Object costringe ogni voter a un instanceof più cast, spostando a runtime controlli che il compilatore regalava, e ripetendoli in ogni voter.
  • Contratto debolissimo (ISP): un’interfaccia che “accetta tutto” comunica pochissimo; passare il soggetto sbagliato si scopre solo a runtime. I generics (AdmissionVoter<T>) aiutano poco: una lista eterogenea di voter riporta a List<AdmissionVoter<?>> e a cast unchecked.
  • Over-generalization (KISS/YAGNI): il voter di Symfony porta con sé astensione e strategie di voto (affirmative/consensus/unanimous), potenza che serve a un framework con soggetti aperti e sconosciuti, non a un dominio con due famiglie note e chiuse.

Quando conviene davvero: il punto di pareggio è quando i soggetti sono molti o aperti, o se stai costruendo un layer di autorizzazione riusabile cross-dominio (è il caso di Symfony). Con due tipi noti, in un linguaggio statico come Java, l’interfaccia separata e tipizzata è più onesta su cosa valida e tiene il compilatore dalla tua parte. In PHP la scelta sarebbe probabilmente opposta: bel promemoria che “clean” dipende anche dal linguaggio.

Il filo conduttore

Mettendo in fila le cinque soluzioni, si vede un’unica direzione di marcia:

Soluzione Dove vivono i controlli Estendere (es. “aggiungo il biglietto”) richiede di… OCP KISS
Soluzione 1 if a memoria, documento dato dal chiamante modificare il metodo del buttafuori ❌ ✅
Soluzione 2 if a memoria, documento recuperato dal buttafuori modificare il metodo ❌ ✅
Soluzione 3 funzioni passate a ogni chiamata aggiungere un’altra funzione alla firma, che si gonfia ancora ➖ ❌
Soluzione 4 un oggetto per controllo, lista nel buttafuori aggiungere un oggetto-controllo ✅ ✅
Soluzione 5 catena dietro una politica aggiungere un controllo o sostituire la politica ✅ ➖

Legenda: ✅ sì, ➖ in parte, ❌ no.

Ogni passo sposta il codice verso l’estensione e lontano dalla modifica, pagando di volta in volta un prezzo in semplicità. Il bello del percorso è proprio vedere quando quel prezzo vale la pena: le Soluzioni 1-2 sono semplici ma rigide (ogni cambiamento è una modifica al cuore); la Soluzione 3 è una falsa partenza, cerca flessibilità ma frammenta le regole e appesantisce l’uso; la Soluzione 4 è il punto di equilibrio, estendibilità vera con un’astrazione minima; la Soluzione 5 è l’optimum strutturale, ma solo se il dominio chiede di sostituire intere politiche o di far crescere molto i controlli.

Come questo si è tradotto nella soluzione finale

Senza entrare nel codice reale, la soluzione adottata è figlia diretta delle Soluzioni 4-5: esiste un controllo per ogni regola, ciascuno con una sola responsabilità e testabile in isolamento (gli oggetti-controllo della Soluzione 4); i controlli sono composti in una catena che esprime “vanno tutti soddisfatti”, e il punto d’ingresso si limita a delegare (l’orchestrazione della Soluzione 5); ogni controllo riceve per iniezione i propri collaboratori ed è predisposto una volta sola, invece di essere ricreato a ogni richiesta; un sottocaso speciale (una famiglia di soggetti con regole proprie, il “VIP” della metafora) è stato tenuto separato dietro la sua astrazione dedicata, invece di forzarlo nella catena generica con parametri che non gli appartengono.

Tutto questo, messo insieme, somiglia a un locale che assembla una volta sola il suo apparato d’ingresso:

// assembled once: standard chain + VIP policy, with collaborators injected
class Venue {
  private final AdmissionPolicy standardPolicy;
  private final VipAdmissionPolicy vipPolicy;

  Venue(Registry registry) {
    // ordinary path: a chain of checks
    this.standardPolicy = new ChainedAdmissionPolicy(
        List.of(new GuestListCheck(), new DocumentCheck(registry), new DressCodeCheck()));
    // VIP path: a single dedicated validator, no chain
    this.vipPolicy = new VipGuestCheck(registry);
  }

  boolean canEnter(Guest guest) {
    if (guest.isVip()) {
      return vipPolicy.allows(guest.asVip());
    }
    return standardPolicy.allows(guest);
  }
}

Aggiungere domani il controllo del biglietto è una riga in più nella lista; il resto non si tocca.

In sintesi: ho preso l’estendibilità della Soluzione 4 e la separazione della Soluzione 5, fermandomi dove il KISS lo consentiva, senza inseguire astrazioni che il dominio non chiedeva.

I test raccontano la stessa storia

Il modo in cui si scrivono i test è forse il segnale più onesto della qualità del design.

Prima (Soluzioni 1-3). Le regole vivono in fondo a un unico flusso. Per verificarne una devi arrivarci: superare i controlli che la precedono e predisporre tutti i collaboratori coinvolti. Il test finisce per parlare dell’esito complessivo, non della singola regola.

@Test
void invalidDocument_guestRejected() {
  // to reach the document check I must first pass the guest list...
  when(guestList.contains(guest)).thenReturn(true);       // context only to "reach the bottom"
  when(registry.lookup("doc-1")).thenReturn(new Document(false));
  Bouncer bouncer = new Bouncer(guestList, registry);

  bouncer.letIn(guest);

  assertThat(bouncer.occupant()).isNull(); // I assert the outcome, not the rule
}

Dopo (Soluzioni 4-5). Ogni controllo si prova isolato, mockando solo il collaboratore che gli serve. E l’orchestratore si prova a parte, mockando l’interfaccia dei controlli: la logica del “tutti d’accordo” si verifica senza alcuna regola reale.

// the single check, isolated: no guest list, no orchestration
@Test
void invalidDocument_theCheckDoesNotAdmit() {
  when(registry.lookup("doc-1")).thenReturn(new Document(false));
  DocumentCheck check = new DocumentCheck(registry);

  assertThat(check.admits(guest)).isFalse();
}

// the orchestrator, isolated from real checks: the checks are mocks of the interface
@Test
void ifOneCheckDoesNotAdmit_thePolicyDoesNotAllow() {
  when(passingCheck.admits(guest)).thenReturn(true);
  when(failingCheck.admits(guest)).thenReturn(false);
  ChainedAdmissionPolicy policy = new ChainedAdmissionPolicy(List.of(passingCheck, failingCheck));

  assertThat(policy.allows(guest)).isFalse();
}

Stessa copertura, ma i test diventano piccoli, mirati e indipendenti: uno per regola, più uno per l’orchestrazione. Quando un test fallisce sai subito quale pezzo è rotto, e aggiungere il controllo del biglietto significa scrivere il test del nuovo controllo, senza rimettere mano ai contesti degli altri.

In sintesi: prima i test erano pochi ma pesanti e accoppiati (tanti mock, contesti complessi, asserzioni sull’esito finale); dopo sono tanti ma leggeri, e per la prima volta si può testare anche l’orchestratore in isolamento, perché è diventato un oggetto a sé.

Morale

Il refactor non è stato “trovare subito la soluzione giusta”, ma far evolvere il design un vincolo alla volta, chiedendosi a ogni passo: questo cambiamento mi rende più facile estendere senza modificare? E quanto mi costa in semplicità? La risposta non è sempre “l’astrazione più potente”: è la più semplice tra quelle che reggono l’estensione che mi serve davvero. Gli scratch raccontano proprio questo viaggio, inclusi i vicoli ciechi, che sono parte del valore.