2009-08-27 7 views
7

Quindi c'è la regola "Fai una cosa" nel libro "Codice pulito". Ma fino a che punto dobbiamo davvero prendere questo.Do One Thing: quanto lontano prendere questa regola?

Per esempio le seguenti dichiarazioni:

Settings.Default.BaudRate = baudRate; 
Settings.Default.COMPort = port; 
Settings.Default.DataBits = dataBits; 
Settings.Default.Handshake = handshake; 
Settings.Default.Parity = parity; 
Settings.Default.ReadTimeout = readTimeout; 
Settings.Default.WriteTimeout = writeTimeout; 
Settings.Default.CommunicationTimeout = communicationTimeout; 
Settings.Default.Save(); 

Ok, sicuro che c'è più di una dichiarazione qui, ma si sente a me come stanno solo facendo una cosa. Salvataggio delle impostazioni.

Ho questo in una singola funzione. Prenderesti davvero questo appartamento e avresti un unico metodo per ogni impostazione?

Quando ti attieni a questa regola e quando no?

+1

Se quelli sono proprietà, allora in sostanza avere un metodo diverso per ciascuno di questi, giusto? Io non sono un ragazzo di C#, ma quelli sembrano oggetti di scena. –

+0

Grazie a tutti per le vostre risposte :-) – TimothyP

risposta

18

Sembra perfettamente valido per me. Il nome del metodo ovvio per quel codice è SaveSettings, che indica che il metodo fa esattamente una cosa. Nulla di cui preoccuparsi.

+0

o saveSettings dipendenti dallo stile di codifica. Questa risposta è però perfetta. – wlashell

+0

Sì, fa *** una cosa ***: Salva le impostazioni. Nessun problema ... – awe

6

Li terrei tutti in una sola funzione SaveSettings() - se li metti ognuno nella propria funzione, dovresti comunque chiamare tutte quelle funzioni da un'altra funzione.

4

Sto assumendo questo riferimento di regole quando dividere una funzione in più sotto-funzioni.

Avete l'idea giusta: il salvataggio delle impostazioni è "una cosa" e potrebbe essere nella sua funzione. Mettere ogni impostazione nella propria funzione sarebbe eccessivo.

Un'altra linea guida che ho ascoltato potrebbe aiutare a comprendere il concetto di "una cosa": se la funzione è più lunga di una o due pagine, è probabilmente meglio scrivere dividendone i contenuti in più sottofunzioni.

18

La sezione successiva del libro, Un livello di astrazione per funzione, fa molto per rispondere a questa domanda. Tutte queste istruzioni sono allo stesso livello di astrazione, quindi questa funzione sta già facendo una cosa, salvando le impostazioni.

+0

Ancora bisogno di arrivare a quella parte: p – TimothyP

6

Sì, ogni metodo dovrebbe fare una sola cosa. Ma cos'è quell'unica cosa?

Dipende dal livello di astrazione del metodo. Un metodo (proprietà) per il salvataggio di una singola impostazione è un'astrazione piuttosto bassa. L'astrazione successiva successiva sarebbe quindi il metodo SaveSettings proposto.

Proprio in cima si ha un unico metodo/funzione main che fa anche solo una cosa: L'intero programma ...

2

sulla nostra squadra cerchiamo di seguire quello scopo per ogni linea guida delle funzioni. Per aiutare gli sviluppatori, abbiamo aggiunto un suggerimento ai nostri standard che considerano il refactoring se una funzione supera le 25 linee. Pertanto, se disponi di 100 righe di proprietà di impostazione del codice, potresti prendere in considerazione la suddivisione in categorie come SaveUserSettings, SaveNetworkSettings, ecc.

L'obiettivo finale è rendere il codice più leggibile. Se hai preso il tuo metodo e lo hai diviso in 20 chiamate che impostano ciascuna una proprietà, penserei che sarebbe più lungo da seguire e supportare.

4

Guardando il codice in isolamento, è difficile da dire.Potresti affermare che la tua funzione fa due cose: aggiornare le impostazioni e salvare le modifiche. Inoltre, come si popolano i valori, vengono passati come argomenti alla funzione (preferibile) o la funzione ottiene i valori stessi (facendo qualcos'altro)?

avrei seguito la Single Responsibility Principle che è riassunta come:

Non ci dovrebbe essere più di un motivo per una classe di cambiare

e può ugualmente essere applicato a metodi. Nel tuo esempio ci sarebbe mai un motivo per aggiornare le impostazioni, ma non salvarle?

+0

Non proprio. Non li aggiornerei mai a meno che non li debba salvare – TimothyP

1

Il codice di esempio andrebbe meglio con la domanda, quando utilizzare le proprietà e quando utilizzare i parametri su un costruttore o un metodo per impostare lo stato dell'oggetto. So che c'è una sezione su questo nel .NET framework guidelines, dove si discute il modello di base del componente (molte proprietà e lo stato interno dei componenti potrebbe non essere valido tra il primo e l'ultimo compito) rispetto all'altro modello (molti parametri del costruttore e parametri del metodo e lo stato interno oggetti è valida dopo ogni riga di codice viene eseguito.

+0

Vedrò quello – TimothyP

4

non ho letto quel libro in particolare, ma per quanto riguarda il concetto di ...

"una cosa" non significa "una riga di codice "." Una cosa "significa che tutto nella funzione deve essere correlato logicamente

Mi piacerebbe cavillare con alcuni dei poster precedenti dicendo che" saveSetti ng "è" una cosa ". Forse è solo una noncuranza con le parole, ma coglierò l'occasione per sottolineare la potenziale trappola. Nel tuo caso, è più simile a "saveCommunicationSettings", che penso si adatta facilmente alla definizione di "una cosa". Se hai aggiunto "Settings.Default.customerLoyaltyDiscount = ..." a quell'elenco, direi che probabilmente sei su un terreno pericoloso, perché ora stai mescolando le impostazioni di comunicazione con le impostazioni di calcolo dei prezzi.

Nella vita reale, decidere quale sia la ragionevole coesione non è una formula ma un giudizio che richiede l'esercizio dell'intelligenza. Una funzione che calcola l'importo totale di un ordine dovrebbe includere il calcolo dell'imposta sulle vendite? Probabilmente sono due cose: il prezzo totale di tutti gli articoli in ordine E calcolare le tasse di vendita. Ma potresti anche sostenere che è solo uno: trovare il prezzo totale dell'ordine, qualunque cosa ciò comporti. In pratica, spesso prendo la decisione in base alla complessità della logica. Se tutto ciò che è necessario per calcolare un totale di un ordine è un semplice ciclo attraverso tutte le voci che sommano i loro prezzi e poi si preleva da una tabella un'aliquota di imposta sulle vendite e si moltiplica, probabilmente farei tutto in un'unica funzione. Se c'è altro in questo, come il sistema su cui sto lavorando in questi giorni, il calcolo del prezzo comprende gli stock rispetto agli ordini personalizzati, la ricerca di tutti i tipi di sconti possibili, l'aggiunta di garanzie, ecc. Ecc. - abbiamo davvero bisogno di romperlo su.

0

Se si desidera suddividere il metodo, prenderei in considerazione la possibilità di interrompere la mappatura dei valori dal salvataggio effettivo dell'oggetto. Si potrebbe dire che la mappatura dei valori è un livello separato di astrazione. Così avresti:

public void save(){ 
    mapSettings(); 
    Settings.Default.Save(); 
} 

private void mapSettings(){ 
    Settings.Default.BaudRate = baudRate; 
    Settings.Default.COMPort = port; 
    Settings.Default.DataBits = dataBits; 
    Settings.Default.Handshake = handshake; 
    Settings.Default.Parity = parity; 
    Settings.Default.ReadTimeout = readTimeout; 
    Settings.Default.WriteTimeout = writeTimeout; 
    Settings.Default.CommunicationTimeout = communicationTimeout; 
} 

Sembra che la mappatura possa essere riutilizzata da qualche altra parte o si potrebbe desiderare. Inoltre, se le impostazioni dei numeri sono cresciute, potrebbero essere ulteriormente suddivise per categoria.

In casi come questo non direi che ne vale la pena, e se l'ho fatto dipende dal mio umore o dalla quantità di nuvole nel cielo. Ma sì, tecnicamente ci sono due livelli che potresti scindere.

0

come sapere se una funzione fa una cosa o più? dipende dal livello delle astrazioni. cerco di descrivere il livello di astrazione negli esempi di traino (libro codice pulito, capitolo 3) vedere questo codice

public function buildPage() { 
    $page = header(); 
    $page .= body(); 
    $page .= footer(); 
    return $page; 
} 

questa funzione fa solo una cosa e tutti i parziali sono parte della frase buildPage

public function sendMail() { 
    $user = $this->getUser(); 
    $this->mailer->content("send this message!") 
     ->to($user->email)->send(); 
} 

questa funzione richiede l'email dell'utente, quindi con l'aiuto della funzione getUser sendMail riesce a ottenere l'email dell'utente. ma $ user = getUser() non sono descritti in Sendmail frase migliori pratiche per questa funzione è

public function sendMail($email,$message) { 
    $this->mailer->content($message)->to($email)->send(); 
} 
Problemi correlati