Mantenibilità della logica booleana - Il nesting è necessario se le istruzioni sono necessarie?

11

Quale di questi è meglio per la manutenibilità?

if (byteArrayVariable != null)
    if (byteArrayVariable .Length != 0)
         //Do something with byteArrayVariable 

o

if ((byteArrayVariable != null) && (byteArrayVariable.Length != 0))
  //Do something with byteArrayVariable 

Preferisco leggere e scrivere il secondo, ma ricordo di aver letto nel codice completo che fare cose del genere è negativo per la manutenibilità.

Questo perché ti stai affidando alla lingua per non valutare la seconda parte di if se la prima parte è falsa e non tutte le lingue lo fanno. (La seconda parte genererà un'eccezione se valutata con un byteArrayVariable nullo.)

Non so se sia davvero qualcosa di cui preoccuparsi o meno, e vorrei un feedback generale sulla domanda.

Grazie.

    
posta Vaccano 19.10.2010 - 22:03
fonte

12 risposte

30

Penso che la seconda forma sia soddisfacente e rappresenta anche più chiaramente ciò che stai cercando di fare. Dici che ...

I recall reading in code complete that doing things like that is bad for maintainability. This is because you are relying on the language to not evaluate the second part of the if if the first part is false and not all languages do that.

Non importa se le lingue tutte lo fanno. Stai scrivendo in una particolare lingua one , quindi importa solo se la lingua quella lo fa. Altrimenti, stai essenzialmente dicendo che non dovresti usare le caratteristiche di una particolare lingua perché altre lingue potrebbero non supportare quelle funzioni, ed è semplicemente sciocco.

    
risposta data 19.10.2010 - 22:10
fonte
16

Sono sorpreso che nessuno l'abbia menzionato (quindi spero di non aver letto male la domanda) - ma entrambi fanno schifo!

L'alternativa migliore sarebbe utilizzare le uscite anticipate (inserendo una dichiarazione di ritorno all'inizio del codice se nessun altro codice dovrebbe essere eseguito). Ciò presuppone che il tuo codice sia relativamente ben refactorato e che ogni metodo abbia uno scopo sintetico.

A quel punto dovresti fare qualcosa come:

if ((byteArrayVariable == null) || (byteArrayVariable.Length != 0)) {
  return; // nothing to be done, leaving
}

// Conditions met, do something

Potrebbe sembrare molto simile alla validazione - ed è esattamente quello che è. Convalida che le condizioni sono state soddisfatte per il metodo di fare la cosa è - entrambe le opzioni che hai offerto hanno nascosto la logica effettiva all'interno dei controlli pre-condizione.

Se il tuo codice contiene molti spaghetti (metodi moooolti, molti if nidificati con logica sparsa tra loro, ecc.) dovresti concentrarti sul problema più grande di ottenere il tuo codice in forma prima dei più piccoli problemi stilistici. A seconda del tuo ambiente e della gravità del pasticcio, ci sono alcuni ottimi strumenti per aiutarti (ad es. Visual Studio ha una funzione di refactoring "Metodo di estrazione").

Come ricorda Jim, c'è ancora spazio per il dibattito su come per strutturare l'uscita anticipata. L'alternativa a ciò che è sopra sarebbe:

if (byteArrayVariable == null) 
  return; // nothing to be done, leaving

if (byteArrayVariable.Length != 0)
  return;

// Conditions met, do something
    
risposta data 21.10.2010 - 03:37
fonte
14

Non perdere tempo a cercare di intercettare ogni condizione non questa . Se riesci a squalificare i dati o le condizioni, fallo il prima possibile.

if (byteArrayVariable == null) Exit;
if (byteArrayVariable.Length == 0) Exit;
// do something

In questo modo puoi adattare il tuo codice molto più facilmente alle nuove condizioni

if (byteArrayVariable == null) Exit;
if (byteArrayVariable.Length == 0) Exit;
if (NewCondition == false) Exit;
if (NewerCondition == true) Exit;
// do something
    
risposta data 06.11.2010 - 19:55
fonte
11

Il tuo esempio è l'esempio perfetto del perché il% co_de nidificato è un approccio errato per la maggior parte delle lingue che supportano correttamente il confronto booleano. Annidare ifs crea una situazione in cui la tua intenzione non è affatto chiara. È richiesto che il secondo ifs segua immediatamente il primo? O è solo perché non hai ancora inserito un'altra operazione?

if ((byteArrayVariable != null) && (byteArrayVariable.Length != 0))
  //Do something with byteArrayVariable 

In questo caso, questi devono essere insieme. Agiscono come una guardia per assicurarsi di non eseguire un'operazione che comporterebbe un'eccezione. Usando% nidificato di co_de, lasci all'interpretazione della persona che ti segue se i due rappresentano una guardia in due parti o qualche altra logica di diramazione. Combinandoli in un singolo if lo sto affermando. È molto più difficile intrufolarsi all'interno di un'operazione dove non dovrebbe essere.

Favorirò sempre una chiara comunicazione delle mie intenzioni sulla stupida affermazione di qualcuno che "non funziona in tutte le lingue". Indovina che cosa ... ifs non funziona in tutte le lingue, vuol dire che non dovrei usarlo durante la codifica in C # o Java e dovrei invece fare affidamento sui valori di ritorno per segnalare quando c'era un problema?

Detto questo, è molto più leggibile invertirlo e tornare al di fuori del metodo se entrambi sono non soddisfatti come dice STW nella loro risposta.

    
risposta data 21.10.2010 - 03:45
fonte
3

In questo caso le tue due frasi sono direttamente correlate, quindi è preferibile la seconda forma.

Se non fossero correlati (ad esempio una serie di clausole di guardia in condizioni diverse), preferirei il 1 ° modulo (o mi piacerebbe rifattorizzare un metodo con un nome che rappresenta ciò che la condizione composta rappresenta in realtà).

    
risposta data 19.10.2010 - 23:25
fonte
1

Se lavori in Delphi, la prima è, in generale, più sicura. (Per l'esempio dato, non c'è molto da guadagnare dall'uso di if annidati.)

Delphi consente di specificare se le espressioni booleane valutano o "cortocircuitano", tramite le opzioni del compilatore. Il modo # 1 (if annidato) ti permette di assicurare che la seconda clausola venga eseguita se e solo se (e rigorosamente dopo ) la prima clausola restituisce true.

    
risposta data 19.10.2010 - 22:28
fonte
1

Il secondo stile può ritorcersi contro VBA poiché se il primo test è se l'oggetto è generato, eseguirà comunque il secondo test e l'errore. Ho appena preso l'abitudine in VBA quando ho a che fare con la verifica degli oggetti per utilizzare sempre il primo metodo.

    
risposta data 19.10.2010 - 23:16
fonte
1

Il secondo modulo è più leggibile, specialmente se stai usando {} blocchi attorno a ogni clausola, perché il primo modulo porterà a nidificati {} blocchi e più livelli di indentazione, che può essere un dolore da leggere (devi conta gli spazi di indentazione per capire dove finisce ogni blocco).

    
risposta data 20.10.2010 - 22:07
fonte
0

Li trovo entrambi leggibili e non accetto l'argomento che dovresti preoccuparti della manutenibilità del codice se dovessi cambiare lingua.

In alcune lingue la seconda opzione funziona meglio quindi sarebbe la scelta più chiara di allora.

    
risposta data 19.10.2010 - 22:10
fonte
0

Sono con te; Lo faccio al secondo modo. Per me, è logicamente più chiaro. La preoccupazione che alcuni linguaggi eseguano la seconda parte anche se la prima valuta come falsa mi sembra un po 'sciocca, come non ho mai scritto in vita mia un programma che girava in "alcune lingue"; il mio codice sembra sempre esistere in una lingua specifica, che può gestire quella costruzione o non può. (E se non sono sicuro che la lingua specifica sia in grado di gestirlo, è qualcosa che ho davvero bisogno di imparare, non è vero?)

Nella mia mente, il pericolo con il primo modo di farlo è che mi rende vulnerabile a inserire accidentalmente del codice al di fuori del blocco% co_de nidificato quando non intendevo farlo. Il che, ammettiamolo, non rappresenta una grande preoccupazione, ma sembra più ragionevole che preoccuparsi del fatto che il mio codice sia indipendente dal linguaggio.

    
risposta data 19.10.2010 - 22:11
fonte
0

Preferisco la seconda opzione, perché è più leggibile.

Se la logica che stai implementando è più complessa (controllando che una stringa non sia nullo e non vuota sia solo un esempio), allora puoi fare un utile refactoring: estrai una funzione booleana. Sono con @mipadi sull'osservazione "Code Complete" ("non importa se tutte le lingue lo fanno ...") Se il lettore del tuo codice non è interessato a guardare all'interno della funzione estratta, allora l'ordine in cui vengono valutati gli operandi booleani diventa una domanda discutibile.

    
risposta data 19.10.2010 - 23:18
fonte
0
if ((byteArrayVariable != null)
 && (byteArrayVariable.Length != 0)) {
    //Do something with byteArrayVariable 

ma fondamentalmente è la seconda opzione.

    
risposta data 04.01.2011 - 02:42
fonte