Dividi il calcolo del valore restituito e dell'istruzione return in metodi a una riga?

L'introduzione della spiegazione delle variabili è un ben noto refactoring che a volte può aiutare a rendere le espressioni complicate più leggibili. Tuttavia, nel caso illustrato,

• la variabile aggiuntiva non "spiega" nulla che non sia chiaro dal nome del metodo circostante
• la frase diventa ancora più lunga, quindi (leggermente) meno leggibile

Inoltre, le versioni più recenti del debugger di Visual Studio possono mostrare il valore restituito di una funzione nella maggior parte dei casi senza introdurre una variabile superflua (ma attenzione, ci sono alcune avvertenze, date un'occhiata a questo post SO più vecchio e le diverse risposte ) .

Quindi, in questo caso specifico, sono d'accordo con te, tuttavia ci sono altri casi in cui una variabile esplicativa può effettivamente migliorare la qualità del codice.

Dati i fatti che:

a) Non c'è alcun impatto sul codice finale in quanto il compilatore ottimizza la variabile.

b) Avere separato migliora la capacità di debug.

Sono arrivato personalmente alla conclusione che è una buona pratica separarli il 99% delle volte.

Non ci sono svantaggi materiali nel farlo in questo modo. L'argomentazione secondo cui il codice è dannoso è un termine improprio, perché il codice eccessivo è un problema banale rispetto al codice illeggibile o difficile da correggere. Inoltre, questo metodo non può di per sé creare codice confuso, che dipende interamente dallo sviluppatore.

Spesso, introdurre una variabile solo per nominare qualche risultato è molto utile quando rende il codice più auto-documentante. In questo caso non è un fattore perché il nome della variabile è molto simile al nome del metodo.

Si noti che i metodi a una linea non hanno alcun valore intrinseco. Se una modifica introduce più righe ma rende il codice più chiaro, si tratta di un buon cambiamento.

Ma in generale, queste decisioni dipendono strongmente dalle tue preferenze personali. Per esempio. Trovo entrambe le soluzioni confuse perché l'operatore condizionale viene utilizzato inutilmente. Avrei preferito un'istruzione if. Ma nella tua squadra potresti aver concordato diverse convenzioni. Quindi fai qualunque cosa suggeriscano le tue convenzioni. Se le convenzioni sono silenziose su un caso come questo, nota che si tratta di una modifica estremamente piccola che non ha importanza nel lungo periodo. Se questo modello si verifica ripetutamente, può iniziare una discussione su come il team desidera gestire questi casi. Ma questo è dividere i capelli tra "codice buono" e "forse un codice un po 'migliore".

In risposta alle tue domande:

La mia domanda è, se è ancora un punto valido per scrivere codice meno chiaro, solo per rendere più facile il debug di uno scorcio?

Sì. In effetti, parte della tua precedente affermazione mi sembra (senza offesa) essere un po 'miope (vedi grassetto sotto) " Il suo argomento era che il primo la variante è più facile da eseguire il debug - che è un valore abbastanza piccolo , dal momento che VisualStudio ci consente un'ispezione molto dettagliata delle istruzioni, quando l'esecuzione viene interrotta a causa di un punto di interruzione. "

Rendere il debug più semplice è (quasi) mai di " piccolo merito " perché con alcune stime il 50% del tempo di un programmatore viene speso per il debug ( Software di debug reversibile ).

Ci sono altri argomenti per la variante con il calcolo di divisione e l'istruzione di ritorno?

Sì. Alcuni sviluppatori sostengono che il calcolo diviso è più facile da leggere. Questo, naturalmente, aiuta nel debug ma aiuta anche quando qualcuno sta cercando di decifrare qualsiasi regola aziendale che il codice possa eseguire o applicare.

NOTA: le regole aziendali possono essere meglio servite in un database poiché possono cambiare spesso. Tuttavia, la codifica chiara in quest'area è ancora fondamentale. ( Come creare un motore di regole aziendali )

Mi piacerebbe andare ancora oltre:

private string GetFormattedValue()
{
    if (format != null) {
        formattedString = string.Format(format, value);
    } else {
        formattedString = value.ToString()
    }
    return formattedString;
}

Perché?

Utilizzare gli operatori ternari per una logica più complessa sarebbe illeggibile, quindi per uno statement più complesso dovresti usare uno stile come sopra. Usando sempre questo stile, il tuo codice è coerente e più facile da analizzare per un essere umano. Inoltre, introducendo questo tipo di coerenza (e utilizzando i lint del codice e altri test) è possibile evitare errori di tipo goto fail .

Un altro vantaggio è che il rapporto sulla copertura del codice ti consente di sapere se hai dimenticato di includere un test per format non è nullo. Questo non sarebbe il caso per l'operatore ternario.

La mia alternativa preferita - se ti trovi in "ottieni un ritorno il più rapidamente possibile" e non contro i ritorni multipli di un metodo:

private string GetFormattedValue()
{
    if (format != null) {
        return string.Format(format, value);
    }

    return value.ToString();
}

Quindi, puoi vedere l'ultimo ritorno per vedere quale è l'impostazione predefinita.

È importante essere coerenti però - e tutti i tuoi metodi seguono l'una o l'altra convenzione.

Non penso che tale tecnica possa essere giustificata dalla necessità di eseguire il debug. Ho incontrato questo approccio me stesso mille volte, e di tanto in tanto continuo a farlo, ma tengo sempre a mente cosa Martin Fowler ha detto sul debug :

People also underestimate the time they spend debugging. They underestimate how much time they can spend chasing a long bug. With testing, I know straight away when I added a bug. That lets me fix the bug immediately, before it can crawl off and hide. There are few things more frustrating or time wasting than debugging. Wouldn't it be a hell of a lot quicker if we just didn't create the bugs in the first place?

Penso che alcune persone si stiano appiccando alle questioni tangenziali alla domanda, come l'operatore ternario. Sì, molte persone lo odiano, quindi forse è bello farlo comunque.

Riguardo al focus della tua domanda, spostando l'istruzione restituita a cui fa riferimento una variabile ...

Questa domanda fa 2 ipotesi che non sono d'accordo con:

1. Che la seconda variante è più chiara o facile da leggere (dico che è vero il contrario) e

2. che tutti utilizzano Visual Studio. Ho usato Visual Studio molte volte e posso usarlo bene, ma di solito sto usando qualcos'altro. Un ambiente di sviluppo che costringe un IDE specifico è uno di cui sarei scettico.

Rompere qualcosa in una variabile denominata rende raramente qualcosa di più difficile da leggere, quasi sempre fa il contrario. La maniera specifica in cui qualcuno lo fa potrebbe causare problemi, come se un overlord di autocomposizione faccia var thisVariableIsTheFormattedResultAndWillBeTheReturnValue = ... , ovviamente questo è male, ma questo è un problema separato. var formattedText = ... va bene.

In questo caso specifico e forse in molti casi poiché stiamo parlando di 1-liners, la variabile non ti dirà molto che il nome della funzione non ti dice già. Pertanto, la variabile non aggiunge tanto. L'argomento di debug potrebbe ancora contenere, ma ancora una volta, in questo caso specifico, non vedo nulla che possa essere il tuo obiettivo durante il debug, e può sempre essere facilmente modificato in seguito se qualcuno ha bisogno di quel formato per il debug o qualsiasi altra cosa.

In generale, e hai chiesto la regola generale (il tuo esempio era proprio questo, un esempio di una forma generalizzata), tutti i punti fatti a favore della variante 1 (2-liner) sono corretti. Queste sono buone linee guida da avere. Ma le linee guida devono essere flessibili. Ad esempio, il progetto su cui sto lavorando ora ha un massimo di 80 caratteri per riga, quindi ho diviso un lotto di linee, ma di solito trovo linee da 81 a 85 caratteri che sarebbero scomodi da dividere o ridurre la leggibilità e li lascio oltre il limite.

Dal momento che è improbabile aggiungere valore, non farei 2 righe per l'esempio specifico indicato. Vorrei fare la variante 2 (la 1-liner) perché i punti non sono abbastanza forti da fare diversamente in questo caso.