Quando il paradigma "Do One Thing" diventa dannoso?

Ovviamente, questo solleva la domanda "Che cosa è una cosa?" Leggere una riga è una cosa e scrivere una riga un'altra? O sta copiando una linea da un flusso all'altro per essere considerata una cosa? O copiare un file?

Non c'è una risposta dura e obiettiva a questo. Tocca a voi. Puoi decidere. Tu devi decidere. L'obiettivo principale del "fare una cosa" è probabilmente produrre un codice che sia il più facile da capire possibile, in modo da poterlo utilizzare come linea guida. Sfortunatamente, anche questo non è oggettivamente misurabile, quindi devi fare affidamento sul tuo istinto e il "WTF?" contare nella revisione del codice .

IMO una funzione composta solo da una singola riga di codice raramente vale la pena. Il tuo printLine() non ha alcun vantaggio sull'uso di std::cout << line << '\n' ¹ direttamente. Se vedo printLine() , devo presumere che faccia quello che dice il suo nome, o cercare e controllare. Se vedo std::cout << line << '\n' , so immediatamente cosa fa, perché questo è il modo canonico di emettere il contenuto di una stringa come una riga su std::cout .

Tuttavia, un altro importante obiettivo del paradigma è consentire il riutilizzo del codice, e questa è una misura molto più obiettiva. Ad esempio, nella tua seconda versione printLines() potrebbe essere facilmente scritto in modo che sia un algoritmo universalmente utile che copia le righe da un flusso all'altro:

void copyLines(std::istream& is, std::ostream& os)
{
  std::string line;
  while( std::getline(is, line) );
    os << line << '\n';
  }
}

Tale algoritmo potrebbe essere riutilizzato anche in altri contesti.

Puoi quindi mettere tutto specifico per questo caso d'uso in una funzione che chiama questo algoritmo generico:

void printFile(const std::string& filePath) {
  std::ifstream file(filePath.c_str());
  printLines(file, std::cout);
}

^{1 _{Nota che ho usato '\n' piuttosto che std::endl . '\n' dovrebbe essere la scelta predefinita per l'output di una nuova riga , std::endl è il caso strano .}}

Avere una funzione fa solo "una cosa" è un mezzo per due fini desiderabili, non un comandamento di Dio:

1. Se la tua funzione fa "una cosa sola", ti aiuterà a evitare la duplicazione del codice e l'aumento dell'API perché sarai in grado di comporre le funzioni per fare cose più complesse invece di avere un'esplosione combinatoria di livelli superiori. livello, funzioni meno componibili.

2. Avendo funzioni solo "una cosa" potrebbe rendere il codice più leggibile. Questo dipende dal fatto che ottieni maggiore chiarezza e facilità di ragionamento disaccoppiando le cose rispetto a quanto perdi per la verbosità, la ricerca indiretta e il sovraccarico concettuale dei costrutti che ti permettono di disaccoppiare le cose.

Pertanto, "una cosa" è inevitabilmente soggettiva e dipende da quale livello di astrazione è rilevante per il tuo programma. Se printLines è pensato come una singola operazione fondamentale e l'unico modo di stampare le linee che ti interessano o che ti preoccupano, allora per i tuoi scopi printLines fa solo una cosa. A meno che tu non trovi la seconda versione più leggibile (io no), la prima versione va bene.

Se inizi a richiedere un maggiore controllo su livelli più bassi di astrazione e finisci con una duplicazione sottile e un'esplosione combinatoria (cioè un printLines per i nomi di file e un printLines completamente separato per fstream oggetti, un printLines per console e un printLines per i file) quindi printLines sta facendo più di una cosa a livello di astrazione a cui tieni.

A questa scala, non importa. L'implementazione a funzione singola è perfettamente ovvia e comprensibile. Tuttavia, l'aggiunta di un po 'più di complessità rende molto interessante dividere l'iterazione dall'azione. Ad esempio, supponiamo di aver bisogno di stampare linee da un insieme di file specificato da un modello come "* .txt". Quindi vorrei separare l'iterazione dall'azione:

printLines(FileSet files) {
   files.each({ 
       file -> file.eachLine({ 
           line -> printLine(line); 
       })
   })
}

Ora l'iterazione del file può essere testata separatamente.

Ho diviso le funzioni per semplificare i test o per migliorare la leggibilità. Se l'azione eseguita su ciascuna riga di dati fosse abbastanza complessa da giustificare un commento, allora sicuramente la dividerei in una funzione separata.

Estrai i metodi quando senti la necessità di un commento per spiegare le cose.

Scrivi metodi che fanno solo quello che dice il loro nome in modo ovvio, o raccontano una storia chiamando metodi con intelligenza.

Anche nel tuo caso semplice, ti mancano i dettagli che il Principio di Responsabilità Unica ti aiuteranno a gestire meglio. Ad esempio, cosa succede quando qualcosa va storto con l'apertura del file. Aggiungendo la gestione delle eccezioni per rafforzare i casi di accesso ai file, aggiungere 7-10 righe di codice alla tua funzione.

Dopo aver aperto il file, non sei ancora al sicuro. Potrebbe essere strappato da te (specialmente se si tratta di un file su una rete), potresti esaurire la memoria, ancora una volta possono verificarsi casi limite che desideri indurire e gonfiare la tua funzione monolitica.

L'unica linea, la stampa sembra abbastanza innocua. Ma man mano che nuove funzionalità vengono aggiunte alla stampante di file (analisi e formattazione del testo, rendering a diversi tipi di display, ecc.) Crescerà e ti ringrazierai più tardi.

L'obiettivo di SRP è quello di consentire di pensare a una singola attività alla volta. È come rompere un grande blocco di testo in più paragrafi in modo che il lettore possa comprendere il punto che stai cercando di attraversare. Ci vuole un po 'più di tempo per scrivere il codice che aderisce a questi principi. Ma in tal modo rendiamo più facile leggere quel codice. Pensa a quanto sarà felice il tuo io futuro quando dovrà rintracciare un bug nel codice e trovarlo ordinatamente partizionato.

Personalmente preferisco il secondo approccio, perché ti risparmia lavoro in futuro e forza il modo di pensare "come farlo in modo generico". Nonostante ciò nel tuo caso la versione 1 è migliore della versione 2 - solo perché i problemi risolti dalla versione 2 sono troppo banali e specifici per fstream. Penso che dovrebbe essere fatto nel modo seguente (inclusa la correzione di bug proposta da Nawaz):

Funzioni di utilità generiche:

void printLine(ostream& output, const string & line) { 
    output << line << endl; 
} 

void printLines(istream& input, ostream& output) { 
    string line; 
    while (getline(input, line)) {
        printLine(output, line); 
    } 
} 

Funzione specifica del dominio:

void printFile(const string & filePath, ostream& output = std::cout) { 
    fstream file(filePath, ios::in); 
    printLines(file, output); 
} 

Ora printLines e printLine possono funzionare non solo con fstream , ma con qualsiasi flusso.

Ogni paradigm , (non solo necessariamente quello che hai citato) da seguire richiede una certa disciplina, e quindi - riducendo la "libertà di parola" - si traduce in un sovraccarico iniziale (almeno solo perché devi impararlo!). In questo senso ogni paradigma può diventare dannoso quando il costo di tale overhead non è sovracompensato dal vantaggio che il paradigma è progettato per mantenere con se stesso.

La vera risposta alla domanda, quindi, richiede una buona capacità di "prevedere" il futuro, come:

• I am adesso richiesto per fare A e B
• Qual è la probabilità, in un vicino futuro mi verrà richiesto di fare anche A- e B+ (cioè qualcosa che assomiglia ad A e B, ma solo un po 'diverso)?
• Qual è la probabilità in un futuro più lontano, che A + diventerà A* o A*- ?

Se quella probabilità è relativamente alta, sarà una buona possibilità se - mentre penso a A e B- penso anche alle loro possibili varianti, quindi per isolare le parti comuni in modo che io possa riutilizzarle.

Se tale probabilità è molto bassa (qualunque variante attorno a A non è essenzialmente nulla più di A stesso), studi come decomporre A molto probabilmente causerà uno spreco di tempo.

Proprio come un esempio, lascia che ti racconti questa storia vera:

Durante la mia vita passata come insegnante, ho scoperto che - per la maggior parte dei progetti degli studenti - praticamente tutti forniscono la propria funzione per calcolare la lunghezza di una stringa C .

Dopo alcune indagini ho scoperto che, essendo un problema frequente, tutti gli studenti hanno avuto l'idea di utilizzare una funzione per questo. Dopo aver detto loro che esiste una funzione di libreria per questo ( strlen ), molti di loro hanno risposto che poiché il problema era così semplice e banale, era più efficace per loro scrivere la propria funzione (2 linee di codice) rispetto alla ricerca del Manuale di libreria C (era il 1984, ha dimenticato il WEB e google!) In rigoroso ordine alfabetico per vedere se c'era una funzione pronta per quello.

Questo è un esempio in cui anche il paradigma "non reinventare la ruota" può diventare dannoso, senza un efficace catalogo delle ruote!

Il tuo esempio va bene per essere utilizzato in uno strumento da buttare via che è necessario ieri per fare qualche compito specifico. O come strumento di amministrazione direttamente controllato da un amministratore. Ora rendilo solido per essere adatto ai tuoi clienti.

Aggiungi la corretta gestione degli errori / delle eccezioni con messaggi significativi. Forse hai bisogno della verifica dei parametri, comprese le decisioni che devono essere prese, ad es. come gestire file non esistenti. Aggiungi funzionalità di registrazione, magari con livelli diversi come informazioni e debug. Aggiungi commenti in modo che i tuoi colleghi sappiano cosa sta succedendo lì. Aggiungi tutte le parti che di solito vengono omesse per brevità e lasciate come esercizio per il lettore quando si forniscono esempi di codice. Non dimenticare i test di unità.

La tua piccola e piuttosto lineare piccola funzione termina improvvisamente in un caos complesso che implora per essere diviso in funzioni separate.

IMO diventa dannoso quando va così lontano che una funzione difficilmente fa qualcosa, ma delegare il lavoro a un'altra funzione, perché questo è un segno che non è più un'astrazione di nulla e la mentalità che porta a tali funzioni è sempre in pericolo di fare cose peggiori ...

Dal post originale

void printLine(const string & line) {
  cout << line << endl;
}

Se sei abbastanza pedante, potresti notare che printLine fa ancora due cose: scrivere la linea per cout e aggiungere un carattere di "fine riga". Alcune persone potrebbero volerlo gestire creando nuove funzioni:

void printLine(const string & line) {
  reallyPrintLine(line);
  addEndLine();
}

void reallyPrintLine(const string & line) {
  cout << line;
}

void addEndLine() {
  cout << endl;
}

Oh no, ora abbiamo ulteriormente aggravato il problema! Ora è persino OBVIOUS che printLine fa DUE cose !!! 1! Non crea molta stupidità per creare le "assurde" soluzioni più assurde che si possano immaginare solo per liberarsi di quell'inevitabile problema che la stampa di una linea consiste nel stampare la linea stessa e aggiungere un carattere di fine riga.

void printLine(const string & line) {
  for (int i=0; i<2; i++)
    reallyPrintLine(line, i);
}

void reallyPrintLine(const string & line, int action) {
  cout << (action==0?line:endl);
}

Risposta breve ... dipende.

Pensa a questo: cosa succede se, in futuro, non vorrai stampare solo sullo standard output, ma su un file.

So cosa è YAGNI, ma sto solo dicendo che potrebbero esserci casi in cui alcune implementazioni sono notoriamente necessarie, ma rimandate. Quindi forse l'architetto o qualsiasi altra cosa sa che funzione deve essere in grado di stampare anche su un file, ma non vuole eseguire l'implementazione in questo momento. Così crea questa funzione extra così, in futuro, è sufficiente modificare l'output in un unico punto. Ha senso?

Se si è certi di aver bisogno solo dell'output nella console, non ha molto senso. Scrivere un "wrapper" su cout << sembra inutile.

L'intera ragione per cui ci sono libri che dedicano capitoli alle virtù del "fare una cosa" è che ci sono ancora sviluppatori là fuori che scrivono funzioni lunghe 4 pagine e livelli di condizionale 6 livelli. Se il tuo codice è semplice e chiaro, hai fatto bene.

Come altri commentatori hanno commentato, fare una cosa è una questione di scala.

Suggerirei anche che l'idea di One Thing sia di fermare la codifica delle persone per effetto collaterale. Questo è esemplificato da accoppiamento sequenziale dove i metodi devono essere chiamati in un ordine particolare per ottenere il risultato 'giusto'.