Ha / può qualcuno sfidare lo zio Bob nel suo amore di rimuovere "bretelle inutili"?

93

Detesto referenziare contenuti paywalled, ma questo video mostra esattamente di cosa sto parlando. Precisamente 12 minuti in Robert Martin guarda a questo:

Edice"Una delle cose che preferisco fare è eliminare le parentesi inutili" mentre lo trasforma in questo:

Moltotempofa,inun'istruzionemoltolontana,mièstatoinsegnatoanonfarloperchérendefacileintrodurreunbugaggiungendoun'altralinearientratapensandochesiacontrollatadaifquandononloè.

PeressereonesticonlozioBob,starifattorizzandounlungometodoJavafinoapiccolefunzioniche,sonod'accordo,sonomoltopiùleggibili.Quandohafinitodicambiarlo,(22.18)sembrachequesto:

Michiedosequestodovrebbeconvalidarelarimozionedelleparentesigraffe.Conoscogiàle best practice . Lo zio Bob può essere sfidato su questo punto? Lo zio Bob ha difeso l'idea?

    
posta candied_orange 04.06.2016 - 01:03
fonte

10 risposte

46

La leggibilità non è poca cosa.

Sono di una mente mista quando si tratta di parentesi graffe che racchiudono un singolo metodo. Personalmente li rimuovo per cose come le dichiarazioni di ritorno a riga singola, ma lasciare fuori queste parentesi ci ha infatti morso molto duramente nell'ultimo posto in cui ho lavorato. Qualcuno ha aggiunto una riga di codice a un'istruzione if senza aggiungere anche le parentesi graffe e, poiché si trattava di C, ha causato l'arresto anomalo del sistema senza alcun preavviso.

Non sfido mai nessuno che sia religioso a usare sempre le parentesi dopo quel piccolo fiasco.

Quindi vedo il vantaggio della leggibilità, ma sono perfettamente consapevole dei problemi che possono sorgere quando esci da quelle parentesi.

Non mi preoccuperei di cercare uno studio o un parere pubblicato da qualcuno. Ognuno ne ha uno (un'opinione), e poiché si tratta di una questione stilistica, un'opinione è semplicemente valida come qualsiasi altra. Pensa a te stesso, valuta i pro e i contro, e crea la tua dannata mente. Se il negozio per cui lavori ha uno standard di codifica che copre questo problema, segui questo.

    
risposta data 04.06.2016 - 02:07
fonte
132

Puoi trovare diverse promozioni pubblicate o rifiuti di stili no-brace su qui o qui o ovunque siano coperti i capannoni per biciclette.

Allontanandosi dai capannoni della bici, ricorda il grande bug OS X / iOS SSL del 2014?

if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
    goto fail;
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    goto fail;
    goto fail;
if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
    goto fail;

Sì, "causato" da blocchi di non parentesi link

Le preferenze possono dipendere dallo stile di controvento. Se dovessi scrivere

if (suiteSetup)
{
    includePage(mode, suiteSetup);
}

Potrei anche essere incline a risparmiare spazio. Ma

if (suiteSetup) {
    includePage(mode, suiteSetup);
}

utilizza solo una linea "extra".

So che non l'hai chiesto, ma se sto lavorando da solo, sono un eretico. Se rimuovo le parentesi, preferisco

if (suiteSetup != null) includePage(mode, suiteSetup);

Non ha lo stesso problema visivo del bug in stile SSL di iOS, ma salva anche più linee "non necessarie" di quelle di Uncle Bob;) Si legge bene se ci si abitua e ha un uso mainstream in Ruby (%codice%). Oh bene, sappi che ci sono molte opinioni.

    
risposta data 04.06.2016 - 06:27
fonte
61

Lo zio Bob ha molti livelli di difesa contro un tale errore che non erano così comuni quando "usare sempre le parentesi" era la saggezza prevalente:

  1. Una strong preferenza personale per i condizionali a linea singola, quindi quelli a più linee spiccano e ricevono un controllo extra.
  2. Un editor che si presenta automaticamente quando inserisci una seconda riga.
  3. Una suite completa di test unitari che esegue costantemente.
  4. Una suite completa di test di integrazione.
  5. Una revisione tra pari fatta prima che il suo codice venga unito.
  6. Riesecuzione di tutti i test da parte di un server di integrazione continua.
  7. Test eseguiti da un reparto di qualità del prodotto.

Penso che se qualcuno pubblicasse una sfida per lo zio Bob, avrebbe una buona confutazione con i punti precedenti. Questo è uno degli errori più facili da prendere in anticipo.

    
risposta data 04.06.2016 - 02:57
fonte
31

Per la maggior parte questa è una preferenza personale, tuttavia ci sono alcune cose da considerare.

Possibili errori

Sebbene si possa sostenere che i bug causati dal dimenticare le parentesi aggiunte sono rari, da quello che ho visto che loro fanno succede occasionalmente (per non dimenticare il famoso IOS goto fail bug). Quindi penso che questo dovrebbe essere un fattore quando si considera lo stile del codice (alcuni strumenti avvertono su fuorviante-indentazione , quindi dipende anche dalla catena di strumenti) .

Codice valido (che legge come potrebbe essere un bug)

Anche supponendo che il tuo progetto non soffra di tali bug, durante la lettura del codice potresti vedere alcuni blocchi di codice che sembrano come potrebbero essere bug - ma non lo sono, prendere parte del tuo mentale cicli.

Iniziamo con:

if (foo)
    bar();

Uno sviluppatore aggiunge un commento utile.

if (foo)
    // At this point we know foo is valid.
    bar();

Più tardi uno sviluppatore si espande su di esso.

if (foo)
    // At this point we know foo is valid.
    // This never fails but is too slow even for debug, so keep disabled.
    // assert(is_valid(foo));
    bar();

O aggiunge un blocco nidificato:

if (foo)
    while (i--) {
        bar(i);
        baz(i);
    }

O usa una macro:

if (foo)
    SOME_MACRO();

"... Poiché le macro possono definire più righe di codice, la macro usa do {...} while (0) per più righe? Dovrebbe perché è nella nostra guida di stile, ma è meglio controllare nel caso in cui!"

Gli esempi sopra sono tutti codici validi, tuttavia più contenuti nel blocco di codice, più devi leggere per assicurarti che non ci siano errori.

Forse il tuo stile di codice definisce che i blocchi multi-linea richiedono una parentesi (non importa cosa, anche se non sono codice) , ma ho visto questo tipo di commenti aggiunti codice di produzione. Quando lo leggi, c'è qualche piccolo dubbio che chiunque abbia modificato quelle righe abbia dimenticato di aggiungere una parentesi graffa, a volte penso che la necessità di ricontrollare funzioni come previsto (specialmente quando si esamina un errore in quest'area del codice ) .

Diff Noise

Un motivo pratico per usare le parentesi per le singole linee è quello di ridurre il rumore diff / .

Cioè, cambiando:

if (foo)
    bar();

A:

if (foo) {
    bar();
    baz();
}

... fa sì che la linea condizionale venga mostrata in un diff come modificato, questo aggiunge un piccolo ma inutile sovraccarico.

  • le linee vengono visualizzate come modificate in code-review, se i tuoi strumenti di diffusione sono basati su parole puoi facilmente vedere che solo la parentesi è cambiata, ma ci vuole più tempo per controllare se la linea non è cambiata affatto .
    Detto questo, non tutti gli strumenti supportano diff, diff (svn, git, hg ... etc) basati su parole, mostreranno come se l'intera linea fosse cambiata, anche con strumenti fantasiosi, a volte potresti aver bisogno di guardare rapidamente su un semplice diff basato sulla linea per vedere cosa è cambiato.
  • strumenti di annotazione (come git blame ) mostreranno la linea come cambiata, rendendo il tracciamento dell'origine di una linea più passaggio per trovare la modifica reale .

Sono entrambi piccoli e dipendono da quanto tempo trascorri in code-review o track-down che commettono linee di codice modificate.

Un inconveniente più tangibile di avere cambiamenti di linee in più in una diff, il loro potenziale più alto che cambia nel codice causerà conflitti che si uniscono e devono essere risolti manualmente .

C'è un'eccezione a questo, per le basi di codice che hanno { sulla propria linea - non è un problema.

L'argomento diff noise non viene mantenuto se scrivi in questo stile:

if (foo)
{
    bar();
    baz();
}

Tuttavia questa non è una convenzione così comune, quindi principalmente aggiungendo alla risposta per completezza (non suggerendo che i progetti dovrebbero usare questo stile) .

    
risposta data 04.06.2016 - 08:11
fonte
15

Anni fa, sono stato introdotto per eseguire il debug del codice C. Il bug era pazzesco, ma alla fine si è ridotto a una frase del tipo:

if (debug)
   foo (4);

E si è scoperto che la persona che l'aveva scritta aveva definito foo come una macro. Una macro con due linee di codice al suo interno. E, naturalmente, solo la prima di quelle due righe era soggetta al if . (Quindi la seconda riga è stata eseguita incondizionatamente.)

Questo può essere assolutamente unico per C e il suo preprocessore - che sostituisce prima della compilazione - ma non l'ho mai dimenticato. Quel genere di cose ti lascia un segno, e perché non giocare sul sicuro - specialmente se usi una varietà di lingue e strumenti e non puoi essere sicuro che tali imbrogli non siano possibili altrove?

Ora indentiamo e uso bretelle in modo diverso da tutti gli altri, a quanto pare. Per una singola riga if , vorrei fare:

if (debug) { foo (4); }

quindi non richiede alcuna riga aggiuntiva per includere le parentesi.

    
risposta data 04.06.2016 - 15:49
fonte
14

"Zio Bob" può avere la sua opinione, puoi avere la tua opinione. Non c'è bisogno di sfidarlo.

Se vuoi un appello all'autorità, prendi Chris Lattner. In Swift, se le affermazioni perdono le parentesi, ma vengono sempre con parentesi graffe. Nessuna discussione, è parte della lingua. Quindi se "Uncle Bob" inizia a rimuovere le parentesi graffe, il codice smette di compilare.

Passare attraverso il codice di qualcun altro e "sbarazzarsi di parentesi inutili" è una cattiva abitudine. Causa solo un lavoro extra quando il codice deve essere rivisto e quando vengono creati inutilmente conflitti. Forse "Uncle Bob" è un programmatore incredibilmente bravo che non ha bisogno di recensioni di codice? Ho perso una settimana della mia vita perché un programmatore in alto ha cambiato "if (p! = NULL)" in "if (! P)" senza una revisione del codice, nascosto nel peggior posto possibile.

Questo è principalmente un dibattito sullo stile innocuo. Le parentesi graffe hanno il vantaggio di poter inserire un'altra riga di codice senza aggiungere graffe. Come una dichiarazione di registrazione, o un commento (se seguito da un commento seguito da una dichiarazione è semplicemente orribile). dichiarazione sulla stessa linea di come se avesse lo svantaggio pratico di avere problemi con molti debugger. Ma fai quello che preferisci.

    
risposta data 05.06.2016 - 09:43
fonte
8

I miei motivi per non rimuovere le parentesi graffe sono:

  1. ridurre l'affaticamento decisionale. Se usi sempre le parentesi graffe, non devi mai decidere se hai bisogno di parentesi o meno.

  2. Riduci il trascinamento dello sviluppo: anche se ti sforzi di eventualmente estrai tutte le linee multiple di logica ai metodi, dovendo convertire un braceless se in uno rinforzato se aggiungere la logica è un fastidioso bit di resistenza allo sviluppo. Quindi c'è lo sforzo di rimuovere le parentesi graffe e lo sforzo di aggiungerle di nuovo quando hai bisogno di più codice. Piccolo, ma fastidioso.

risposta data 06.06.2016 - 08:08
fonte
0

Un giovane collaboratore ha affermato che le coppie che consideriamo ridondanti e superflue sono di fatto utili a lui. Non ricordo esattamente il motivo, ma se gli permette di scrivere un codice migliore più veloce, questo è il solo motivo per mantenerli.

Per quanto ci riguarda, abbiamo concordato un compromesso sul fatto che metterli su una sola riga non renda a me tali precondizioni un così poco comprensibili. Per es.

if (!param) { throw invalid_argument (blahblah); }
if (n < 2) { return 0; }
// real code for function starts here...

Sottolineo anche che queste precondizioni introduttive sono spesso dichiarazioni di flusso di controllo per il corpo (ad esempio un return ) quindi la paura di aggiungere una seconda affermazione che è destinata a essere nelle stesse condizioni e dimenticare di scrivere le parentesi è discutibile . Una seconda affermazione sotto la condizione sarebbe comunque un codice morto e non ha senso.

Sospetto che il problema di scioltezza nella lettura sia causato dal fatto che il parser cerebrale della persona è cablato per avere le parentesi graffe come parte dell'istruzione condizionale. Questa non è una rappresentazione accurata della grammatica del C ++, ma potrebbe essere un effetto collaterale dell'apprendimento di alcune altre lingue prima, in questo caso.

    
risposta data 05.06.2016 - 21:19
fonte
-1

Avendo visto quel video proprio ora, ho notato che l'eliminazione delle parentesi rende più facile vedere dove il codice deve ancora essere rifatto. Se hai bisogno di parentesi graffe, probabilmente il tuo codice sarà un po 'più pulito.

Detto questo, quando sei in una squadra, l'eliminazione delle parentesi graffe probabilmente porterà a un comportamento imprevisto un giorno. Dico di tenerli, e ti risparmierai un sacco di mal di testa quando le cose vanno male.

    
risposta data 05.10.2016 - 11:47
fonte
-3

I was taught not to do this because it makes it easy to introduce a bug by adding another indented line thinking it's controlled by the if when it's not.

Se il tuo codice è ben testato dall'unità , questo tipo di "bug" esploderebbe in faccia.

Pulire il codice evitando ogni parentesi inutile e brutta è una pratica che seguo.

    
risposta data 06.06.2016 - 03:22
fonte

Leggi altre domande sui tag