È una buona pratica scrivere un metodo che ottiene qualcosa e controlla il valore? [duplicare]

Question

È una buona pratica scrivere un metodo che ottiene qualcosa e controlla il valore? [duplicare]

#1 da (3 voti)
#2 da (1 voti)
#3 da (0 voti)
#4 da (0 voti)
#5 da (0 voti)

1

Occasionalmente devo scrivere metodi come questo:

string GetReportOutputDirectoryAndMakeSureExist()
{
    string path = Path.Combine ( ... ) //whatever logic 
    if(!Directory.Exists(path)) Directory.Create(path);
    return path;
}

o

string GetAndVerifyExistenceOfReportConfigurationPath()
{
    string path = Path.Combine ( ... ) //whatever logic to find the configuration
    if(!File.Exists(path)) throw new Exception("Report configuration not found");
    return path;
}

o

Customer GetCustomerAndVerifyActive(int id)
{
    Customer customer = Db.GetCustomerById(id);
    if(!customer.IsActive) throw new Exception("Customer is not active");
    return customer;
}

È una buona pratica? Mi è stato detto che normalmente non è una buona idea che un metodo faccia più cose, o che un metodo abbia effetti collaterali (come la creazione di directory). Ma se divido, per esempio l'ultimo metodo per GetCustomer (id) e VerifyActive (cliente), dovrò fare:

var customer = GetCustomer(id);
VerifyActive(customer);

consecutivamente in diversi luoghi, e penso che contenga la violazione di DRY. E 'questa una buona idea? Qualche idea su come aiutare con i nomi dei metodi lunghi?

coding-style programming-practices methods .net

posta Louis Rhys 16.05.2013 - 09:14

fonte

5 risposte

Leggi altre domande sui tag coding-style programming-practices methods .net

Posso prendere in prestito parti di un progetto con licenza Apache per qualcosa che non è open-source? Si tratta di un framework MVC?

score 3 · Answer 1

Ciò che i tuoi metodi stanno facendo non è affatto irragionevole. È solo un peccato che i loro nomi siano un po 'pieni di parole.

Ad esempio, dove hai un metodo GetCustomerAndVerifyActive , sicuramente potresti chiamare questo GetActiveCustomer invece, ed è più facile per gli occhi? Il nuovo nome non toglie nulla a ciò che effettivamente fa il metodo.

In sintesi, penso che i nomi dei metodi dovrebbero riflettere ciò che il metodo fa ad un livello elevato, non linea per riga. E sicuramente in .net hai gli abbellimenti /// che ti permettono di specificare il metodo in modo più dettagliato.

Esempio

/// <summary>
/// Returns an Active customer
/// </summary>
/// <param name="id">the customer's id</param>
/// <returns>the active customer</returns>
/// <remarks>Note this will throw an exception if the customer doesn't exist, or is inactive</remarks>
Customer GetActiveCustomer(int id)
{
    Customer customer = Db.GetCustomerById(id);
    if(!customer.IsActive) throw new Exception("Customer is not active");
    return customer;
}

score 1 · Answer 2

Qui sono in gioco due principi generali:

responsabilità singola
interfacce intelligenti

È sempre una buona idea che un'unica entità, metodo o classe abbia una responsabilità, ma l'interfaccia per qualsiasi funzione o classe dovrebbe essere facile da usare.

Se deve sempre essere verificato e soprattutto se non ha senso utilizzare l'oggetto, che si ottiene senza verifica, è necessario verificarlo nel metodo get.

nomi così lunghi per variabili non leggibili e non necessari. È molto sensato usare semplicemente getVarName e lasciare che faccia il controllo.

score 0 · Answer 3

L'unico punto di "una sola responsabilità" è rendere i programmi più facili da capire per i lettori umani. Calcolare un certo percorso e assicurarsi che esista può essere visto come due diverse responsabilità, ma sono certamente strettamente correlate; Non vedo il rischio di confondere la tua comprensione qui, in particolare finché la routine di ricerca rimane così breve come ora.

Certamente è meglio che dover ricordare che il percorso, che probabilmente vorrai manipolare dopo averlo recuperato, potrebbe non esistere: ciò richiederebbe di scrivere il doppio del codice in ogni posto in cui usa la routine, con una perdita netta netta.

Una buona cartina tornasole per queste domande è: se garantire l'esistenza non è il lavoro di questa routine, di chi è il lavoro? Qualsiasi utilità che si scriverà per lo scopo dovrebbe prima chiamare la routine di calcolo del percorso, in modo che assomiglino a una coppia di gemelli congiunti - né realmente con alcuno scopo se non in combinazione con l'altro. Può essere una buona pratica per refactoring tali routine "gemelle" in metodi separati comunque per una migliore leggibilità, ma non quando entrambi sono piccoli come questi sono. (Nota: questa è un'opinione. Sono consapevole che ci sono persone che rifattorizzano tutto quando è più lungo di una riga.)

Ho assunto qui che in realtà è necessario garantire l'esistenza di questo percorso su ogni accesso. Se questo non è il caso, allora hai due compiti diversi:

Ipoteticamente, se avessi un, dove lo metterei?
Dammi un luogo dove posso metterlo subito!

Probabilmente questi due meriterebbero metodi diversi.

score 0 · Answer 4

I primi due esempi e il terzo sono abbastanza diversi.

Nei primi due esempi si controlla prima una condizione (se il percorso esiste) e solo dopo si costruisce un oggetto da restituire. Questo è assolutamente accettabile.

Quello che fai nel terzo esempio è recuperare i dati e controllare se i dati sono validi. Penso che questo sia troppo complicato. Sarebbe meglio prima controllare se il cliente è attivo (magari aggiungendo un metodo al livello DB per quello) prima di recuperarlo.

Anche lanciare un'eccezione se un oggetto non può essere creato è troppo complicato da IMHO. Puoi restituire null invece di throwing ed exception, quindi controlla semplicemente null invece di avere blocchi try-catch.

Per quanto riguarda i nomi dei metodi, si descrivono verbalmente tutti i passaggi nei metodi _ recupero, convalida, controllo. Possono essere facilmente semplificati:

GetReportOutputDirectory anziché GetReportOutputDirectoryAndMakeSureExist
GetReportConfigurationPath anziché GetAndVerifyExistenceOfReportConfigurationPath
GetActiveCustomer anziché GetCustomerAndVerifyActive

Di solito, i metodi devono restituire dati validi, che possono essere garantiti dal controllo preliminare di una condizione. Quindi non direi che i metodi fanno due cose: restituiscono un oggetto valido, il che è abbastanza ragionevole.

score 0 · Answer 5

I am told that it is normally a good idea for a method to do two things, or for a method to have side-effects (like creating directory).

Bene, è normale che i metodi fanno più di una cosa, cioè ci siamo abituati, ma non è consigliabile (in generale).

Nei tuoi esempi particolari vedo una linea molto sottile tra comfort / singola responsabilità e leggibilità / separazione delle preoccupazioni. Il problema principale che vedo, tuttavia, è che hai codificato il comportamento dei metodi nei loro nomi. Per esempio. GetReportOutputDirectoryAndMakeSureExist fa esattamente questo nel codice. Quando cambi leggermente il comportamento (ad esempio aggiungi un file fittizio in ogni nuova directory), dovresti cambiare anche il nome del metodo.

Il mio suggerimento (e sembra che non sia l'unico) è iniziare con la modifica dei nomi dei metodi. Ad esempio GetReportOutputDirectoryAndMakeSureExist potrebbe essere rinominato in initReportOutputDirectory . Quindi dovresti dare un'occhiata ai dintorni dei metodi: è un metodo davvero usato che spesso si trova nel progetto? O solo all'interno di una singola classe? Trovo spesso che un'altra segregazione abbia più senso, quando si guarda indietro.