Leggibilità e manutenibilità, caso speciale di scrittura di chiamate di funzioni annidate

57

Il mio stile di codifica per chiamate di funzioni annidate è il seguente:

var result_h1 = H1(b1);
var result_h2 = H2(b2);
var result_g1 = G1(result_h1, result_h2);
var result_g2 = G2(c1);
var a = F(result_g1, result_g2);

Recentemente sono passato a un reparto in cui il seguente stile di codifica è molto utilizzato:

var a = F(G1(H1(b1), H2(b2)), G2(c1));

Il risultato del mio modo di codificare è che, in caso di una funzione di arresto anomalo, Visual Studio può aprire il dump corrispondente e indicare la linea in cui si verifica il problema (sono particolarmente preoccupato per le violazioni di accesso).

Temo che, in caso di crash dovuto allo stesso problema programmato nel primo modo, non sarò in grado di sapere quale funzione ha causato l'arresto.

D'altra parte, maggiore è l'elaborazione che si esegue su una riga, maggiore è la logica su una pagina, che migliora la leggibilità.

La mia paura è corretta o mi sfugge qualcosa, e in generale, che è preferibile in un ambiente commerciale? Leggibilità o manutenibilità?

Non so se è rilevante, ma stiamo lavorando in C ++ (STL) / C #.

    
posta Dominique 22.02.2018 - 12:27
fonte

9 risposte

111

Se ti sei sentito in dovere di espandere un solo rivestimento come

 a = F(G1(H1(b1), H2(b2)), G2(c1));

Non ti biasimo. Non è solo difficile da leggere, è difficile eseguire il debug.

Perché?

  1. È denso
  2. Alcuni debugger evidenzieranno il tutto solo in una volta
  3. È privo di nomi descrittivi

Se lo espandi con risultati intermedi ottieni

 var result_h1 = H1(b1);
 var result_h2 = H2(b2);
 var result_g1 = G1(result_h1, result_h2);
 var result_g2 = G2(c1);
 var a = F(result_g1, result_g2);

ed è ancora difficile da leggere. Perché? Risolve due dei problemi e ne introduce un quarto:

  1. È denso
  2. Alcuni debugger evidenzieranno l'intera operazione in una sola volta
  3. È privo di nomi descrittivi
  4. È ingombra di nomi non descrittivi

Se lo espandi con nomi che aggiungono un significato nuovo, buono, semantico, ancora meglio! Un buon nome mi aiuta a capire.

 var temperature = H1(b1);
 var humidity = H2(b2);
 var precipitation = G1(temperature, humidity);
 var dewPoint = G2(c1);
 var forecast = F(precipitation, dewPoint);

Ora almeno questo racconta una storia. Risolve i problemi ed è chiaramente migliore di qualsiasi altra offerta qui, ma richiede di trovare i nomi.

Se lo fai con nomi privi di significato come result_this e result_that perché semplicemente non riesci a pensare a nomi validi, preferirei che tu ci risparmi la confusione di significato senza nome e la espandi usando alcuni buoni spazi bianchi:

int a = 
    F(
        G1(
            H1(b1), 
            H2(b2)
        ), 
        G2(c1)
    )
;

È altrettanto leggibile, se non di più, di quello con i nomi dei risultati privi di significato (non che questi nomi di funzioni siano così eccezionali).

  1. È denso
  2. Alcuni debugger evidenzieranno l'intera operazione in una sola volta
  3. È privo di nomi descrittivi
  4. È ingombra di nomi non descrittivi

Quando non riesci a pensare a nomi di qualità, è buono come lo è.

Per qualche motivo i debugger amano le nuove linee quindi dovresti trovare che il debugging non sia difficile:

Seciònonbastasse,immaginaG2()èstatochiamatoinpiùdiunpostoepoièsuccesso:

Exception in thread "main" java.lang.NullPointerException at composition.Example.G2(Example.java:34) at composition.Example.main(Example.java:18)

Penso che sia bello il fatto che ogni G2() chiamata sia sulla propria linea, questo stile ti porta direttamente alla chiamata all'origine della chiamata principale.

Quindi, per favore, non usare i problemi 1 e 2 come una scusa per attaccarci al problema 4. Usa dei buoni nomi quando riesci a pensarli. Evita nomi privi di significato quando non puoi.

Gare di leggerezza in Orbit commenta giustamente sottolinea che queste funzioni sono artificiali e hanno dei nomi scadenti. Ecco un esempio di come applicare questo stile ad un codice dal selvaggio:

var user = db.t_ST_User.Where(_user => string.Compare(domain,  
_user.domainName.Trim(), StringComparison.OrdinalIgnoreCase) == 0)
.Where(_user => string.Compare(samAccountName, _user.samAccountName.Trim(), 
StringComparison.OrdinalIgnoreCase) == 0).Where(_user => _user.deleted == false)
.FirstOrDefault();

Odio guardare quel flusso di rumore, anche quando non è necessario il word wrapping. Ecco come appare sotto questo stile:

var user = db
    .t_ST_User
    .Where(
        _user => string.Compare(
            domain, 
            _user.domainName.Trim(), 
            StringComparison.OrdinalIgnoreCase
        ) == 0
    )
    .Where(
        _user => string.Compare(
            samAccountName, 
            _user.samAccountName.Trim(), 
            StringComparison.OrdinalIgnoreCase
        ) == 0
    )
    .Where(_user => _user.deleted == false)
    .FirstOrDefault()
;

Come puoi vedere, ho trovato che questo stile funziona bene con il codice funzionale che si sta spostando nello spazio orientato agli oggetti. Se riesci a trovare un buon nome per farlo in stile intermedio, allora più potere a te. Fino ad allora lo sto usando. Ma in ogni caso, per favore, trova un modo per evitare nomi di risultati privi di significato. Mi fanno male agli occhi.

    
risposta data 22.02.2018 - 14:54
fonte
50

On the other hand, the more processing you put on a line, the more logic you get on one page, which enhances readability.

Non sono affatto d'accordo con questo. Basta guardare i tuoi due esempi di codice come errato:

var a = F(G1(H1(b1), H2(b2)), G2(c1));

si sente leggere. "Leggibilità" non significa densità di informazione; significa "facile da leggere, capire e mantenere".

A volte, il codice è semplice e ha senso usare una singola riga. Altre volte, farlo rende più difficile leggere, senza alcun vantaggio evidente oltre a riempire di più su una riga.

Tuttavia, ti chiamerei anche per affermare che "diagnosi facili da diagnosticare" significa che il codice è facile da mantenere. Il codice che non si blocca è molto più facile da mantenere. "Facile da mantenere" è ottenuto principalmente tramite il codice, facile da leggere e comprendere, supportato da una buona serie di test automatizzati.

Quindi, se trasformi una singola espressione in una multi-linea con molte variabili solo perché spesso il tuo codice si arresta in modo anomalo e hai bisogno di maggiori informazioni di debug, smetti di farlo e rendi il codice più robusto. Dovresti preferire la scrittura di codice che non richiede il debug su codice facile da eseguire il debug.

    
risposta data 22.02.2018 - 12:45
fonte
25

Il tuo primo esempio, il modulo di assegnazione singola, è illeggibile perché i nomi scelti non hanno alcun significato. Potrebbe essere un artefatto di tentare di non rivelare informazioni interne da parte tua, il vero codice potrebbe andare bene sotto questo aspetto, non possiamo dire. In ogni caso, è prolisso a causa della densità di informazioni estremamente bassa, che in genere non si presta a una facile comprensione.

Il tuo secondo esempio è condensato in un grado assurdo. Se le funzioni avevano nomi utili, potrebbe essere buona e ben leggibile perché non c'è troppo di esso, ma come-è confuso nella direzione opposta.

Dopo aver introdotto nomi significativi, potresti verificare se una delle forme sembra naturale, o se c'è una zona centrale d'oro da utilizzare.

Ora che hai codice leggibile, la maggior parte dei bug sarà ovvia, e gli altri almeno ti nasconderanno un po 'più difficile.

    
risposta data 22.02.2018 - 13:08
fonte
17

Come sempre, quando si parla di leggibilità, l'errore è agli estremi . Puoi prendere qualsiasi buon consiglio di programmazione, trasformarlo in una regola religiosa e usarlo per produrre codice completamente illeggibile. (Se non mi credi su questo, dai un'occhiata a questi due IOCCC vincitori borsanyi e goren e dai un'occhiata a quanto diversamente usano le funzioni per rendere il codice completamente illeggibile suggerimento: Borsanyi usa esattamente una funzione, goren molto, molto di più ...)

Nel tuo caso, i due estremi sono 1) usando solo le espressioni di espressione singola e 2) unendo tutto in dichiarazioni grandi, concise e complesse. Entrambi gli approcci adottati rendono il codice illeggibile.

Il tuo compito, come programmatore, è di trovare un equilibrio . Per ogni affermazione che scrivi, è il tuo compito rispondere alla domanda: "Questa affermazione è facile da comprendere e serve a rendere leggibile la mia funzione?"

Il punto è che non esiste una singola complessità di istruzione misurabile che possa decidere, ciò che è buono da includere in una singola affermazione. Prendi ad esempio la linea:

double d = sqrt(square(x1 - x0) + square(y1 - y0));

Questa è un'affermazione piuttosto complessa, ma qualsiasi programmatore che valga la pena spendere dovrebbe essere in grado di capire immediatamente cosa fa. È un modello abbastanza noto. In quanto tale, è molto più leggibile rispetto all'equivalente

double dx = x1 - x0;
double dy = y1 - y0;
double dxSquare = square(dx);
double dySquare = square(dy);
double dSquare = dxSquare + dySquare;
double d = sqrt(dSquare);

che interrompe il modello noto in un numero apparentemente privo di significato di semplici passaggi. Tuttavia, l'affermazione dalla tua domanda

var a = F(G1(H1(b1), H2(b2)), G2(c1));

mi sembra troppo complicato, anche se si tratta di un'operazione inferiore al calcolo della distanza . Naturalmente, questa è una conseguenza diretta di me che non so nulla su F() , G1() , G2() , H1() o H2() . Potrei decidere diversamente se ne sapessi di più. Ma questo è precisamente il problema: la complessità consigliabile di una dichiarazione dipende strongmente dal contesto e dalle operazioni coinvolte. E tu, come programmatore, sei colui che deve dare un'occhiata a questo contesto e decidere cosa includere in una singola affermazione. Se ti interessa la leggibilità, non puoi scaricare questa responsabilità in alcune regole statiche.

    
risposta data 23.02.2018 - 15:07
fonte
14

@Dominique, penso che nell'analisi della tua domanda tu stia commettendo l'errore che "leggibilità" e "manutenibilità" sono due cose separate.

È possibile avere un codice che è manutenibile ma illeggibile? Viceversa, se il codice è estremamente leggibile, perché dovrebbe diventare non mantenibile per essere leggibile? Non ho mai sentito di nessun programmatore che abbia giocato questi fattori l'uno contro l'altro, dovendo scegliere l'uno o l'altro!

In termini di decidere se utilizzare variabili intermedie per chiamate di funzioni annidate, nel caso di 3 variabili date, chiamate a 5 funzioni separate, e alcune chiamate nidificate 3 in profondità, tenderei a usare almeno alcuni variabili intermedie per spezzarle, come hai fatto tu.

Ma di certo non mi spingo a dire che le chiamate di funzione non devono mai essere nidificate. È una questione di giudizio nelle circostanze.

Direi che i seguenti punti si basano sul giudizio:

  1. Se le funzioni chiamate rappresentano operazioni matematiche standard, sono più in grado di essere annidate rispetto a funzioni che rappresentano una logica di dominio oscuro i cui risultati sono imprevedibili e non possono essere necessariamente valutati mentalmente dal lettore.

  2. Una funzione con un singolo parametro è più capace di partecipare a un nido (come funzione interna o esterna) rispetto a una funzione con più parametri. Le funzioni di missaggio di diverse entità a diversi livelli di nidificazione sono inclini a lasciare il codice simile all'orecchio di un maiale.

  3. Un insieme di funzioni che i programmatori sono abituati a vedere espresse in un modo particolare - forse perché rappresenta una tecnica matematica standard o un'equazione, che ha un'implementazione standard - potrebbe essere più difficile leggere e verificare se è suddiviso in variabili intermedie.

  4. Un piccolo insieme di chiamate di funzione che eseguono funzionalità semplici ed è già chiaro da leggere, ed è quindi suddiviso in modo eccessivo e atomizzato, può essere più difficile da leggere di quello che non è stato affatto suddiviso.

risposta data 22.02.2018 - 15:37
fonte
4

Entrambi sono non ottimali. Considera commenti.

// Calculating torque according to Newton/Dominique, 4th ed. pg 235
var a = F(G1(H1(b1), H2(b2)), G2(c1));

O funzioni specifiche piuttosto che generali:

var a = Torque_NewtonDominique(b1,b2,c1);

Quando decidi quali risultati indicare, tieni a mente i costi (copia vs riferimento, valore-valore vs valore-r), leggibilità e rischio, singolarmente per ciascuna affermazione.

Ad esempio, non vi è alcun valore aggiunto dallo spostamento di semplici conversioni di unità / tipo sulle proprie linee, poiché sono facili da leggere ed estremamente improbabili da fallire:

var radians = ExtractAngle(c1.Normalize())
var a = Torque(b1.ToNewton(),b2.ToMeters(),radians);

Riguardo alla tua preoccupazione di analizzare i crash dump, la validazione degli input è di solito molto più importante - è probabile che il crash effettivo avvenga all'interno di queste funzioni piuttosto che sulla linea che li chiama, e anche se non lo è, di solito non è necessario ha detto esattamente dove le cose sono esplose. È molto più importante sapere dove le cose hanno iniziato a cadere a pezzi, piuttosto che sapere dove hanno fatto esplodere, il che è ciò che cattura la validazione degli input.

    
risposta data 23.02.2018 - 15:43
fonte
1

La leggibilità è la maggior parte della manutenibilità. Dubitare di me? Scegli un grande progetto in una lingua che non conosci (perferibilmente sia il linguaggio di programmazione che la lingua dei programmatori), e guarda come procederesti a refactoring ...

Metterei la leggibilità come da qualche parte tra 80 e 90 di manutenibilità. L'altro 10-20% è quanto sia utile per il refactoring.

Detto questo, passerai effettivamente 2 variabili alla tua funzione finale (F). Quelle 2 variabili sono create usando 3 altre variabili. Sarebbe stato meglio passare b1, b2 e c1 in F, se F esiste già, quindi creare D che fa la composizione per F e restituisce il risultato. A quel punto è solo questione di dare a D un buon nome, e non importa quale stile usi.

Su un non correlato, dici che più logica sulla pagina aiuta la leggibilità. Non è corretto, la metrica non è la pagina, è il metodo e la logica LESS che contiene un metodo, più leggibile è.

Leggibile significa che il programmatore può tenere la logica (input, output e algoritmo) nella propria testa. Più lo fa, meno un programmatore può capirlo. Leggi sulla complessità ciclomatica.

    
risposta data 23.02.2018 - 03:00
fonte
1

Indipendentemente che tu sia in C # o C ++, finché sei in una build di debug, una possibile soluzione è il wrapping delle funzioni

var a = F(G1(H1(b1), H2(b2)), G2(c1));

Puoi scrivere espressioni oneline, e ancora essere puntato dove il problema è semplicemente guardando la traccia dello stack.

returnType F( params)
{
    returnType RealF( params);
}

Naturalmente, se si chiama la stessa funzione più volte nella stessa riga non si può sapere quale funzione, tuttavia è ancora possibile identificarla:

  • Guardando i parametri della funzione
  • Se i parametri sono identici e la funzione non ha effetti collaterali, due chiamate identiche diventano 2 chiamate identiche ecc.

Questo non è un proiettile d'argento, ma non è così male a metà strada.

Per non parlare del fatto che il raggruppamento di funzioni può persino essere più utile per la leggibilità del codice:

type CallingGBecauseFTheorem( T b1, C b2)
{
     return G1( H1( b1), H2( b2));
}

var a = F( CallingGBecauseFTheorem( b1,b2), G2( c1));
    
risposta data 25.02.2018 - 09:45
fonte
1

A mio parere, il codice di autocodifica è migliore sia per la manutenibilità che per la leggibilità, indipendentemente dalla lingua.

La dichiarazione di cui sopra è densa, ma "auto-documentante":

double d = sqrt(square(x1 - x0) + square(y1 - y0));

Se suddiviso in più fasi (più facile per i test, sicuramente) perde tutto il contesto come sopra indicato:

double dx = x1 - x0;
double dy = y1 - y0;
double dxSquare = square(dx);
double dySquare = square(dy);
double dSquare = dxSquare + dySquare;
double d = sqrt(dSquare);

E ovviamente l'utilizzo di nomi di variabili e funzioni che indicano chiaramente il loro scopo è inestimabile.

Anche i blocchi "se" possono essere buoni o cattivi per l'auto-documentazione. Questo è male perché non puoi forzare facilmente le prime 2 condizioni per testare il terzo ... non sono tutte correlate:

if (Bill is the boss) && (i == 3) && (the carnival is next weekend)

Questo rende più senso "collettivo" ed è più facile creare condizioni di test:

if (iRowCount == 2) || (iRowCount == 50) || (iRowCount > 100)

E questa affermazione è solo una sequenza casuale di caratteri, vista da una prospettiva di auto-documentazione:

var a = F(G1(H1(b1), H2(b2)), G2(c1));

Osservando la suddetta affermazione, la manutenibilità è ancora una sfida importante se le funzioni H1 e H2 alterano entrambe le stesse "variabili di stato del sistema" invece di essere unificate in una singola funzione "H", perché qualcuno alla fine modificherà l'H1 senza nemmeno pensare c'è una funzione H2 da guardare e potrebbe rompere H2.

Credo che una buona progettazione del codice sia molto impegnativa perché non esistono regole rigide che possano essere rilevate e applicate sistematicamente.

    
risposta data 27.02.2018 - 10:41
fonte

Leggi altre domande sui tag