Clean code: conseguenze di metodi brevi con pochi parametri

12

Recentemente durante una revisione del codice mi sono imbattuto in codice, scritto da un nuovo collega, che contiene un motivo con un odore. Sospetto che le decisioni del mio collega siano basate su regole proposte dal famoso libro del Codice Pulito (e forse anche da altri libri simili).

Ho capito che il costruttore di classi è interamente responsabile della creazione di un oggetto valido e che il suo compito principale è l'assegnazione delle proprietà (private) di un oggetto. Potrebbe naturalmente accadere che i valori di proprietà opzionali possano essere impostati con metodi diversi dal costruttore di classi, ma tali situazioni sono piuttosto rare (sebbene non necessariamente errate, a condizione che il resto della classe tenga conto dell'opportunità di tale proprietà). Questo è importante perché consente di garantire che l'oggetto sia sempre in uno stato valido.

Tuttavia, nel codice che ho incontrato, la maggior parte dei valori di proprietà sono effettivamente impostati da altri metodi rispetto al costruttore. I valori che risultano dai calcoli sono assegnati alle proprietà da utilizzare all'interno di diversi metodi privati in tutta la classe. L'autore utilizza apparentemente le proprietà di classe come se fossero variabili globali che dovrebbero essere accessibili in tutta la classe, invece di parametrizzare questi valori alle funzioni che ne hanno bisogno. Inoltre, i metodi della classe dovrebbero essere chiamati in un ordine specifico, perché la classe non farà molto altrimenti.

Sospetto che questo codice sia stato ispirato dal consiglio di mantenere i metodi brevi (< = 5 linee di codice), per evitare elenchi di parametri di grandi dimensioni (< 3 parametri) e che i costruttori non devono lavorare (come eseguire un calcolo di qualche tipo che è essenziale per la validità dell'oggetto).

Ora naturalmente potrei argomentare contro questo modello se posso dimostrare che tutti i tipi di errori non definiti possono sorgere quando i metodi non vengono chiamati in un ordine specifico. Tuttavia, prevedo che la risposta a questo sarà l'aggiunta di convalide che verificano che le proprietà devono essere impostate una volta che i metodi sono chiamati che hanno bisogno di quelle proprietà da impostare.

Preferirei piuttosto proporre di cambiare completamente il codice, in modo che la classe diventi una stampa blu su un oggetto reale, piuttosto che una serie di metodi che dovrebbero essere chiamati (proceduralmente) in un ordine specifico.

Sento che il codice che ho incontrato odora. In effetti, credo che esista una distinzione abbastanza chiara su quando salvare un valore in una proprietà di classe e quando metterlo in un parametro per un metodo diverso da usare - non credo davvero che possano essere alternativi tra loro . Sto cercando le parole per questa distinzione.

    
posta user2180613 21.10.2016 - 00:03
fonte

3 risposte

12

Come qualcuno che ha letto Clean Code e visto la serie Clean Coders, più volte, e spesso insegna e istruisce altre persone nello scrivere codice più pulito, posso davvero garantire che le tue osservazioni siano corrette - le metriche che fai notare sono tutte citate nel libro.

Tuttavia, il libro continua a fare altri punti che dovrebbero essere applicati accanto alle linee guida che hai indicato. Questi sono stati apparentemente ignorati nel codice che hai a che fare. Questo potrebbe essere successo perché il tuo collega è ancora in fase di apprendimento, nel qual caso, per quanto sia necessario indicare gli odori del loro codice, è bene ricordare che lo stanno facendo in buona volontà, imparando e cercando di scrivere codice migliore.

Clean Code suggerisce che i metodi dovrebbero essere brevi, con il minor numero possibile di argomenti. Ma seguendo queste linee guida, propone che dobbiamo seguire i principi dell'OLID, rafforzare la coesione e ridurre l'accoppiamento .

Il S in SOLID sta per Single Responsibility Principle, che afferma che un oggetto dovrebbe essere responsabile solo di una cosa. "Cosa" non è un termine molto preciso, quindi le descrizioni di questo principio variano selvaggiamente. Tuttavia, lo zio Bob, l'autore di Clean Code, è anche la persona che ha coniato questo principio, descrivendolo come: "Raccogli le cose che cambiano per gli stessi motivi. Separa quelle cose che cambiano per ragioni diverse". Continua dicendo cosa intende per motivi per cambiare qui e qui (una spiegazione più lunga qui sarebbe troppo). Se questo principio è stato applicato alla classe con cui hai a che fare, è molto probabile che i pezzi che trattano i calcoli siano separati da quelli che trattano lo stato di mantenimento, dividendo la classe in due o più, a seconda di quanti < em> motivi per cambiare quei calcoli hanno.

Inoltre, le classi Clean dovrebbero essere cohesive , il che significa che la maggior parte dei suoi metodi usa la maggior parte dei suoi attributi. Come tale, una classe massimamente coesiva è quella in cui tutti i metodi usano tutti i suoi attributi; come esempio, in un'app grafica potresti avere una classe Vector con attributi Point a e Point b , dove gli unici metodi sono scaleBy(double factor) e printTo(Canvas canvas) , entrambi operanti su entrambi gli attributi. Al contrario, una classe minimamente coesiva è quella in cui ogni attributo è usato in un solo metodo, e mai più di un attributo è usato da ciascun metodo. In media, una classe presenta "gruppi" non coesivi di parti coesive - cioè alcuni metodi usano attributi a , b e c , mentre il resto usa c e d - ciò significa che se noi dividi la classe in due, finiamo con due oggetti coesivi.

Infine, le classi Clean dovrebbero ridurre il accoppiamento il più possibile. Anche se ci sono molti tipi di accoppiamenti che vale la pena discutere qui, sembra che il codice a disposizione soffra principalmente di accoppiamento temporale , dove, come hai sottolineato, i metodi dell'oggetto funzioneranno come previsto solo quando vengono chiamati nell'ordine corretto. E come le due linee guida sopra citate, le soluzioni a questo di solito comportano la suddivisione della classe in due o più oggetti coesi . La strategia di splitting in questo caso di solito coinvolge modelli come Builder o Factory, e in casi altamente complessi, State-Machines.

Il TL; DR: le linee guida sul codice pulito che il tuo collega ha seguito sono buone, ma solo quando seguono anche i principi, le pratiche e i modelli rimanenti menzionati dal libro. La versione Clean della "classe" che stai vedendo verrebbe divisa in più classi, ciascuna con una sola responsabilità, metodi coesivi e nessun accoppiamento temporale. Questo è il contesto in cui hanno senso i piccoli metodi e gli argomenti poco-meno.

    
risposta data 21.10.2016 - 04:06
fonte
9

It is my understanding that the class constructor is entirely responsible for the creation of a valid object and that its main task is the assignment of an object's (private) properties.

Di solito è responsabile mettere l'oggetto in uno stato iniziale valido, sì; altre proprietà o metodi possono quindi cambiare lo stato in un altro stato valido.

However, in the code that I encountered, most property values are actually set by other methods than the constructor. Values that result from calculations are assigned to properties to be used inside several private methods throughout the class. The author seemingly uses class properties as if they were global variables that should be accessible throughout the class, instead of parameterizing these values to the functions that need them. Additionally, the methods of the class should be called in a specific order, because the class won't do much otherwise.

Oltre ai problemi di leggibilità e manutenibilità a cui alludi, sembra che ci siano più fasi di flusso / trasformazione dei dati in corso all'interno della classe stessa, il che potrebbe indicare che la classe sta fallendo il principio della singola responsabilità .

suspect that this code has been inspired by the advice to keep methods short (<=5 lines of code), to avoid large parameter lists (<3 parameters) and that constructors must not do work (such as performing a calculation of some sort that is essential for the validity of the object).

Seguire alcune linee guida di codifica mentre ignorando gli altri spesso porta a codice stupido. Se vogliamo evitare che un costruttore lavori, per esempio, il modo sensato sarebbe di solito di fare il lavoro prima della costruzione e di trasmettere il risultato di quel lavoro al costruttore. (Un argomento per questo approccio potrebbe essere che stai evitando di dare alla tua classe due responsabilità: il lavoro della sua inizializzazione e il suo "lavoro principale", qualunque esso sia.)

Nella mia esperienza, rendere classi e metodi piccoli raramente è qualcosa che devo tenere a mente come una considerazione separata - piuttosto, segue in qualche modo naturalmente da una singola responsabilità.

I would however rather propose to completely change the code, so that the class becomes a blue print to an actual object, rather than a series of methods which should be called (procedurally) in a specific order.

Probabilmente saresti corretto a farlo. Non c'è niente di sbagliato nello scrivere codice procedurale semplice; c'è un problema nell'abusare il paradigma OO per scrivere codice procedurale offuscato.

I believe there exists a rather clear distinction as to when to save a value in a class property and when to put it into a parameter for a different method to use - I don't really believe they can be alternatives to one another. I am looking for the words for this distinction.

Di solito , non dovresti mettere un valore in un campo come modo per passarlo da un metodo all'altro; un valore in un campo dovrebbe essere una parte significativa dello stato di un oggetto in un dato momento. (Posso pensare ad alcune eccezioni valide, ma non a quelle in cui tali metodi sono pubblici o in cui esiste una dipendenza di ordine di cui un utente della classe dovrebbe essere a conoscenza)

    
risposta data 21.10.2016 - 00:32
fonte
5

Molto probabilmente stai scortando contro lo schema sbagliato qui. Piccole funzioni e bassa aritm sono raramente problematiche da sole. Il vero problema qui è l'accoppiamento che causa la dipendenza dall'ordine tra le funzioni, quindi cerca modi per indirizzare quel senza a buttar via i benefici delle piccole funzioni.

Il codice parla più delle parole. Le persone ricevono questo tipo di correzioni molto meglio se puoi effettivamente fare parte del refactoring e mostrare il miglioramento, magari come esercizio di programmazione di coppia. Quando faccio questo, trovo che sia più difficile di quanto pensassi per ottenere il design giusto, bilanciando tutti i criteri.

    
risposta data 21.10.2016 - 03:00
fonte

Leggi altre domande sui tag