Coding: concisione / efficienza rispetto alla leggibilità

4

Sono abbastanza nuovo in C # e sto cercando di imparare le migliori pratiche. Ho dovuto affrontare molte situazioni nell'ultima settimana in cui ho bisogno di scegliere tra codice più lungo + più semplice o codice più corto che combina diverse azioni in una singola istruzione. Quali sono alcuni standard che i codificatori veterani usano quando provano a scrivere un codice chiaro e conciso? Ecco un esempio di codice che sto scrivendo con due opzioni. Quale è preferibile?

A)

    if (ncPointType.StartsWith("A"))//analog points
    {
        string[] precisionString = Regex.Split(unitsParam.Last(), ", ");
        precision = int.Parse(precisionString[1]);
    }
    else
    {
        precision = null;
    }

B)

    if (ncPointType.StartsWith("A"))//analog points
        precision = int.Parse(Regex.Split(unitsParam.Last(), ", ")[1]);
    else
        precision = null;

C)

    precision = ncPointType.StartsWith("A") == true ? 
            int.Parse(Regex.Split(unitsParam.Last(), ", ")[1]) : 
            null;
    
posta MatthewHagemann 15.05.2014 - 23:42
fonte

5 risposte

16

È come scrivere in inglese. Raramente conciso è una brutta cosa, purché il lettore possa vedere tutte le informazioni di cui hanno bisogno per capire l'intento.

La ragione per cui l'opzione C non è perfettamente leggibile non è perché è TOO concisa, è perché non è abbastanza concisa. Guarda di nuovo:

precision = ncPointType.StartsWith("A") == true ? 
            int.Parse(Regex.Split(unitsParam.Last(), ", ")[1]) : 
            null;

E se lo dicesse?

precision = ncPointType.StartsWith("A") ? SecondToken(unitsParam.Last()) : null

Mi sembra abbastanza leggibile. È molto chiaro che se ncPointType inizia con un A, imposteremo la precisione sul secondo token nell'ultimo parametro di unitsParam. Altrimenti lo imposteremo su null.

I dettagli della tokenizzazione sono irrilevanti per la persona che legge questa riga di codice. Ma se vogliono saperlo, possono osservare il metodo, che dirà.

public int SecondToken(string param)
{
    return int.Parse(GetTokens(param)[1]);
}

Ha senso. Molto chiaro. E poi ...

public string[] GetTokens(string param)
{
    return Regex.Split(param, ", ");
}

Ora stai per dire che questo è molto più di una qualsiasi delle tue opzioni. E questo è. Ma pensa a ciascun metodo come parte di un glossario di termini. Stai creando una lingua mentre vai.

Non farlo è come cercare di scrivere una storia sui tuoi animali domestici, dopo aver rimosso le parole cane e gatto dal dizionario.

Ci sono molte parole nella definizione di cane; ancora di più nel dizionario nel suo complesso. Ma non avere quella definizione per cane significa invece dire "mammiferi carnivori addomesticati" e la tua storia diventa meno leggibile.

Detto questo, come ho accennato all'inizio, c'è un punto in cui perdi informazioni. Ad esempio:

precision = ncPointType.IsOk() ? GetIt() : Dont();

Questo è un passo troppo lungo. Per battere l'ultima parte dell'utilità da un'analogia: è come scrivere una storia sui tuoi animali domestici, riferendoli sempre a "loro".

    
risposta data 16.05.2014 - 00:50
fonte
2

Da Pulisci codice :

Each line represents an expression or a clause, and each group of lines represents a complete thought. Those thoughts should be separated from each other with blank lines.

Inoltre, trovo che gli operatori ternari possono essere illeggibili se le linee sono troppo lunghe; quindi andrei con B .

Tuttavia, anche da Clean Code, vorrei dividere il condizionale nel suo metodo.

It only takes a few seconds of thought to explain most of your intent in code. In many cases it's simply a matter of creating a function that says the same thing as the comment you want to write.

Considera, che è più leggibile, questo:

if (ncPointType.StartsWith("A"))//analog points
    precision = int.Parse(Regex.Split(unitsParam.Last(), ", ")[1]);
else
    precision = null;

o questo:

if (isAnalog(ncPointType))
    precision = int.Parse(Regex.Split(unitsParam.Last(), ", ")[1]);
else
    precision = null;
    
risposta data 15.05.2014 - 23:53
fonte
1

Altro codice. Più righe di codice non si traducono in più istruzioni eseguite.

In tutti i tuoi esempi mi aspetterei che il compilatore producesse codice identico (salvo alcune variazioni nelle mappature dei numeri di riga da utilizzare nella diagnostica). Mi aspetto anche un benchmark sulle tre varianti per ottenere risultati identici.

I compilatori sono pezzi di software molto intelligenti scritti da un gran numero di persone molto intelligenti per diversi anni.

Il tuo programma sarà gestito da una sola persona (probabilmente te stesso) che potrebbe non essere così intelligente e potrebbe avere solo pochi minuti per leggere il tuo codice e risolvere un problema.

Quindi la tua priorità dovrebbe essere il povero sucker che ha letto il tuo codice, non il super intelligente compilatore di ottimizzazione che renderà tutti i vecchi rifiuti in un codice macchina ragionevolmente efficiente.

La chiave qui è la leggibilità e la comprensione. Qualunque cosa renda più chiara la tua intenzione è il codice migliore. Puoi farlo dividendo linee di codice denso o, al contrario, puoi farlo riducendo molte righe di codice a una singola affermazione concisa - è tutto questione di giudizio.

    
risposta data 16.05.2014 - 03:24
fonte
0

Non essendo a conoscenza del resto del tuo codice, preferisco chiaramente l'opzione A.

A) Nel caso in cui desideri eseguire il debug, avrai un facile accesso a precisionString

B) Non così buono, il valore di ritorno dell'analisi delle regexp non sarà facilmente accessibile nel debug. Anche l'assenza di {} è pericolosa: l'aggiunta di un'istruzione (es: una riga di debug) interromperà la logica

C) In realtà è troppo prolisso sarebbe anche accessibile per avere:

precision = ncPointType.StartsWith("A") /* Analog unit */ ? 
        int.Parse(Regex.Split(unitsParam.Last(), ", ")[1]) : 
        null;

(senza l'accesso facile al valore di ritorno della divisione regexp)

    
risposta data 18.05.2014 - 19:29
fonte
0

Se puoi avere sia la leggibilità che la concisione e l'efficienza, sarebbe l'ideale. Tuttavia, sono d'accordo sul fatto che a volte avere troppa concisione può rendere il codice illeggibile. Ad esempio, spesso suddivido il mio codice in più righe e attribuisco un nome a ciascuna parte intermedia dei miei calcoli, piuttosto che schiacciare calcoli complessi in un'unica riga di codice.

Personalmente, preferisco le istruzioni if-else sulla notazione in C. È più facile fare punti di interruzione se è necessario testare un punto specifico. Se si schiaccia il codice in troppe poche righe, è più difficile da testare. Inoltre, poiché l'istruzione if-else è ben indentata, un lettore può vedere la struttura del codice senza nemmeno leggere nulla.

Suppongo che la mia regola personale sia questa: "Se dovessi spiegare il mio blocco di codice a qualcuno nel più semplice inglese possibile, il codice rifletterà le mie parole?" Se trovo che dovrei rompere il codice e spiegarne ogni parte, allora il codice stesso deve essere scomposto.

E come ha detto James Anderson, molto probabilmente non farà molta differenza in termini di prestazioni. Di solito, se sono costretto a scegliere, sceglierei la leggibilità rispetto alla concisione.

Consiglierei di prendere in considerazione la regola 90/10. Il 90% del tuo rallentamento è solitamente concentrato nel 10% del tuo codice. Alcune persone preferiscono l'efficienza al punto in cui il codice è illeggibile. Quindi, il prossimo sviluppatore deve dedicare così tanto tempo alla decifrazione del codice che non ha tempo per ottimizzare il 10% del codice che sta effettivamente causando problemi di efficienza. Rendi più facile per la prossima persona sfogliare il tuo codice, in modo che abbia più tempo per ottimizzare ciò che effettivamente DEVE essere ottimizzato.

    
risposta data 28.10.2016 - 22:31
fonte

Leggi altre domande sui tag