Altri blocchi aumentano la complessità del codice? [chiuso]

17

Ecco un esempio semplificato molto . Questa non è necessariamente una domanda specifica per la lingua, e ti chiedo di ignorare i molti altri modi in cui la funzione può essere scritta e le modifiche che possono essere apportate ad essa. . Il colore è di un tipo unico

string CanLeaveWithoutUmbrella()
{
    if(sky.Color.Equals(Color.Blue))
    {
        return "Yes you can";
    }
    else
    {
        return "No you can't";
    }
}

Molte persone che ho incontrato, ReSharper e questo tizio (il cui commento mi ha ricordato I ' Ho cercato di chiederglielo per un po ') raccomanderei di rifattorizzare il codice per rimuovere il blocco else lasciando questo:

(Non riesco a ricordare quello che la maggior parte ha detto, potrei non averlo chiesto diversamente)

string CanLeaveWithoutUmbrella()
{
    if(sky.Color.Equals(Color.Blue))
    {
        return "Yes you can";
    }
    return "No you can't";
}

Domanda: c'è un aumento della complessità introdotto non includendo il blocco else ?

Ho l'impressione che else più direttamente dichiari l'intento, affermando il fatto che il codice in entrambi i blocchi è direttamente correlato.

Inoltre, trovo che posso evitare sottili errori nella logica, specialmente dopo le modifiche al codice in un secondo momento.

Prendi questa variante del mio esempio semplificato (ignorando il fatto che l'operatore or poiché questo è un esempio volutamente semplificato):

bool CanLeaveWithoutUmbrella()
{
    if(sky.Color != Color.Blue)
    {
        return false;
    }
    return true;
}

Qualcuno può ora aggiungere un nuovo blocco if basato su una condizione dopo il primo esempio senza riconoscere immediatamente che la prima condizione sta ponendo un vincolo sulla propria condizione.

Se fosse presente un blocco else , chiunque avesse aggiunto la nuova condizione sarebbe stato costretto a spostare il contenuto del blocco else (e se in qualche modo lo ignorassero, l'euristica mostrerà che il codice non è raggiungibile, cosa che fa non nel caso di un if che ne costringe un altro).

Naturalmente ci sono altri modi in cui l'esempio specifico dovrebbe essere comunque definito, il che impedisce a questa situazione, ma è solo un esempio.

La lunghezza dell'esempio che ho dato potrebbe distorcere l'aspetto visivo di questo, quindi supponiamo che lo spazio occupato tra parentesi sia relativamente insignificante rispetto al resto del metodo.

Ho dimenticato di menzionare un caso in cui sono d'accordo con l'omissione di un blocco else, ed è quando utilizzo un blocco if per applicare un vincolo che deve essere logicamente soddisfatto per tutti i seguenti codici, come un null-check (o qualsiasi altro guardiano).

    
posta Selali Adobor 06.09.2014 - 02:59
fonte

16 risposte

26

A mio avviso, è preferibile il blocco esplicito. Quando vedo questo:

if (sky.Color != Blue) {
   ...
}  else {
   ...
}

So che mi sto occupando di opzioni mutualmente esclusive. Non ho bisogno di leggere cosa c'è dentro i blocchi se poterlo dire.

Quando vedo questo:

if (sky.Color != Blue) {
   ...
} 
return false;

Sembra, a prima vista, che restituisca false indipendentemente e abbia un effetto collaterale opzionale che il cielo non sia blu.

Come la vedo io, la struttura del secondo codice non riflette ciò che effettivamente fa il codice. Questo è male. Invece, scegli la prima opzione che riflette la struttura logica. Non voglio dover controllare la logica return / break / continue in ogni blocco per conoscere il flusso logico.

Perché qualcuno preferirebbe l'altro è un mistero per me, spero che qualcuno prenderà la posizione opposta e mi illuminerà con la loro risposta.

I forgot to mention a case in which I agree with the omission of an else block, and is when using an if block to apply a constraint that must be logically satisfied for all following code, such as a null-check (or any other guards).

Direi che le condizioni di guardia sono a posto nel caso in cui tu abbia lasciato la strada felice. Si è verificato un errore o un errore che impedisce la continuazione dell'esecuzione ordinaria. Quando ciò accade, devi generare un'eccezione, restituire un codice di errore o qualsiasi altra informazione appropriata per la tua lingua.

Poco dopo la versione originale di questa risposta, codice come questo è stato scoperto nel mio progetto:

Foobar merge(Foobar left, Foobar right) {
   if(!left.isEmpty()) {
      if(!right.isEmpty()) {
          Foobar merged = // construct merged Foobar
          // missing return merged statement
      }
      return left;
   }
   return right;
}

Non inserendo il rendimento all'interno, il fatto che mancasse una dichiarazione di ritorno è stato trascurato. Se fosse stato impiegato altro, il codice non sarebbe stato compilato. (Naturalmente, la preoccupazione più grande che ho è che i test scritti su questo codice sono stati piuttosto negativi per non rilevare questo problema.)

    
risposta data 06.09.2014 - 03:50
fonte
22

Il motivo principale per cui ho rimosso il blocco else che ho trovato è l'indentazione in eccesso. Il cortocircuito dei blocchi else consente una struttura di codice molto più piatta.

Molte dichiarazioni di if sono essenzialmente delle guardie. Stanno impedendo il dereferenziamento di puntatori nulli e altri "non farlo!" errori. E portano a una rapida interruzione della funzione corrente. Ad esempio:

if (obj === NULL) {
    return NULL;
}
// further processing that now can assume obj attributes are set

Ora può sembrare che una clausola else sia molto ragionevole qui:

if (obj === NULL) {
    return NULL;
}
else if (obj.color != Blue) {
   // handle case that object is not blue
}

E infatti, se questo è tutto ciò che stai facendo, non è molto più rientrato rispetto all'esempio sopra. Tuttavia, raramente si tratta di strutture di dati reali a più livelli (una condizione che si verifica sempre durante l'elaborazione di formati comuni come JSON, HTML e XML). Quindi, se vuoi modificare il testo di tutti i bambini di un determinato nodo HTML, dì:

elements = tree.xpath(".//p");
if (elements === NULL) {
    return NULL;
}
p = elements[0]
if ((p.children === NULL) or (p.children.length == 0)) {
    return NULL;
}
for (c in p.children) {
    c.text = c.text.toUpperCase();
}
return p;

Le guardie non aumentano il rientro del codice. Con una clausola else , tutto il lavoro effettivo inizia a spostarsi a destra:

elements = tree.xpath(".//p");
if (elements === NULL) {
    return NULL;
}
else {
    p = elements[0]
    if ((p.children === NULL) or (p.children.length == 0)) {
        return NULL;
    }
    else {
        for (c in p.children) {
            c.text = c.text.toUpperCase();
        }
        return p;
    }
}

Questo sta iniziando ad avere un significativo movimento verso destra, e questo è un esempio piuttosto banale, con solo due condizioni di guardia. Ogni guardia aggiuntiva aggiunge un altro livello di indentazione. Il codice reale che ho scritto elaborando XML o consumando dettagliati feed JSON può facilmente impilare 3, 4 o più condizioni di guardia. Se usi sempre else , finirai con i livelli rientrati di 4, 5 o 6 pollici. Forse di più. E questo contribuisce sicuramente ad un senso di complessità del codice, e più tempo speso a capire cosa si allinea con cosa. Lo stile veloce, piatto, ad uscita anticipata elimina parte di quella "struttura in eccesso" e sembra più semplice.

Addendum I commenti su questa risposta mi hanno fatto capire che potrebbe non essere stato chiaro che la gestione NULL / vuota è non l'unica ragione per i condizionali della guardia. Non vicino! Mentre questo semplice esempio si concentra sulla gestione NULL / vuota e non contiene altri motivi, la ricerca di strutture profonde come XML e AST per contenuti "rilevanti", ad esempio, ha spesso una lunga serie di test specifici per eliminare nodi irrilevanti e poco interessanti casi. Una serie di test "se questo nodo non è pertinente, restituisce" causerà lo stesso tipo di deriva verso destra che le protezioni NULL / vuote faranno. E in pratica, i test di rilevanza successivi sono frequentemente accoppiati con protezioni NULL / vuote per le strutture dati corrispondenti più profonde ricercate - un doppio smacco per deriva verso destra.

    
risposta data 06.09.2014 - 04:59
fonte
4

Quando vedo questo:

if(something){
    return lamp;
}
return table;

Vedo un tipico idiom . Un modello comune, che nella mia testa si traduce in:

if(something){
    return lamp;
}
else return table;

Poiché questo è un idioma molto comune nella programmazione, il programmatore che è abituato a vedere questo tipo di codice ha una "traduzione" implicita nella loro testa ogni volta che lo vedono, che lo "converte" nel significato appropriato.

Non ho bisogno dell'esplicativo else , la mia testa 'lo mette lì' per me perché ha riconosciuto il modello nel suo complesso . Capisco il significato dell'intero schema comune, quindi non ho bisogno di piccoli dettagli per chiarirlo.

Ad esempio, leggi il seguente testo:

Hailettoquesto?

IloveParisinthespringtime

Seècosì,tisbagli.Leggiiltestoancora.Ciòcheèeffettivamentescrittoè

IloveParisinthethespringtime

Cisonodue"le" parole nel testo. Allora perché ti sei perso uno dei due?

È perché il tuo cervello ha visto un modello comune e immediatamente lo ha tradotto nel suo significato noto. Non aveva bisogno di leggere il dettaglio dell'intera cosa per dettaglio per capire il significato, perché già riconosceva il modello dopo averlo visto. Questo è il motivo per cui ha ignorato l'extra "il ".

La stessa cosa con l'idioma di cui sopra. I programmatori che hanno letto molto codice sono abituati a vedere questo modello , di if(something) {return x;} return y; . Non hanno bisogno di un explciit else per capire il suo significato, perché il loro cervello 'lo mette lì per loro'. Lo riconoscono come un pattern con un significato noto: "ciò che viene dopo la parentesi di chiusura verrà eseguito solo come% implicito% del% precedente% co_de".

Quindi, secondo me, il else non è necessario. Ma usa solo ciò che sembra più leggibile.

    
risposta data 06.09.2014 - 19:35
fonte
2

Aggiungono ingombro visivo nel codice, quindi sì, potrebbero aggiungere complessità.

Tuttavia, è vero anche il contrario, un eccessivo refactoring per ridurre la lunghezza del codice può anche aggiungere complessità (non in questo semplice caso, ovviamente).

Sono d'accordo con l'affermazione che il blocco else è intento. Ma la mia conclusione è diversa, perché stai aggiungendo complessità al tuo codice per raggiungere questo obiettivo.

Non sono d'accordo con la tua affermazione che ti permette di evitare errori sottili nella logica.

Vediamo un esempio, ora dovrebbe restituire true solo se il cielo è blu e c'è umidità elevata, l'umidità non è alta o se il cielo è rosso. Ma poi, sbagli una parentesi e la metti nel else sbagliato:

bool CanLeaveWithoutUmbrella() {
    if(sky.Color == Color.Blue)
    {
        if (sky.Humidity == Humidity.High) 
        {
            return true;
        } 
    else if (sky.Humidity != Humidity.High)
    {
        return true;
    } else if (sky.Color == Color.Red) 
    {
            return true;
    }
    } else 
    {
       return false;
    }
}

Abbiamo visto tutti questi errori stupidi nel codice di produzione e può essere molto difficile da rilevare.

Suggerirei quanto segue:

Trovo che questo sia più facile da leggere, perché posso vedere a colpo d'occhio che il valore predefinito è false .

Inoltre, l'idea di forzare a spostare il contenuto di un blocco per inserire una nuova clausola, è incline agli errori. Questo in qualche modo si riferisce al principio aperto / chiuso (il codice dovrebbe essere aperto per estensione ma chiuso per la modifica). Stai forzando a modificare il codice esistente (ad esempio, il coder potrebbe introdurre un errore nel bilanciamento delle parentesi).

Infine, stai guidando le persone a rispondere in modo predeterminato che pensi sia quello giusto. Per esempio, si presenta un esempio molto semplice, ma in seguito si dichiara che le persone non dovrebbero prenderlo in considerazione. Tuttavia, la semplicità dell'esempio influisce sull'intera domanda:

  • Se il caso è semplice come quello presentato, la mia risposta sarebbe di omettere qualsiasi clausola if else .

    return (sky.Color == Color.Blue);

  • Se il caso è un po 'più complicato, con varie clausole, risponderei:

    bool CanLeaveWithoutUmbrella () {

    if(sky.Color == Color.Blue && sky.Humidity == Humidity.High) {
        return true;
    } else if (sky.Humidity != Humidity.High) {
        return true;
    } else if (sky.Color == Color.Red) {
        return true;
    }
    
    return false;
    

    }

  • Se il caso è complicato e non tutte le clausole restituiscono true (forse restituiscono un double), e voglio introdurre qualche breakpoint o comando di logging, considererei qualcosa del tipo:

    double CanLeaveWithoutUmbrella() {
    
        double risk = 0.0;
    
        if(sky.Color == Color.Blue && sky.Humidity == Humidity.High) {
            risk = 1.0;
        } else if (sky.Humidity != Humidity.High) {
            risk = 0.5;
        } else if (sky.Color == Color.Red) {
            risk = 0.8;
        }
    
        Log("value of risk:" + risk);
        return risk;
    

    }

  • Se non stiamo parlando di un metodo che restituisce qualcosa, ma invece di un blocco if-else che imposta una variabile o chiama un metodo, allora sì, includerei una clausola else finale.

Pertanto, la semplicità dell'esempio è anche un fattore da considerare.

    
risposta data 06.09.2014 - 17:57
fonte
1

Sebbene il caso if-else descritto abbia la maggiore complessità, nella maggior parte (ma non in tutte) le situazioni pratiche non importa molto.

Anni fa, quando stavo lavorando a software utilizzato per l'industria aeronautica (doveva passare attraverso un processo di certificazione), la tua era una domanda che si presentava. Si è scoperto che c'era un costo finanziario per questa affermazione "else" in quanto aumentava la complessità del codice.

Ecco perché.

La presenza del "else" ha creato un terzo caso implicito che doveva essere valutato, ovvero che né il "se" né il "altro" venivano presi. Potresti pensare (come all'epoca) a WTF?

La spiegazione è andata così ...

Da un punto di vista logico, ci sono solo le due opzioni: "se" e "altro". Tuttavia, ciò che stavamo certificando era il file oggetto che il compilatore stava generando. Pertanto, quando c'era un "altro", doveva essere eseguita un'analisi per confermare che il codice di diramazione generato era corretto e che un misterioso terzo percorso non veniva visualizzato.

Confrontalo con il caso in cui c'è solo un "se" e nessun "altro". In tal caso, ci sono solo due opzioni: il percorso "se" e il percorso "non-se". Due casi da valutare sono meno complessi di tre.

Spero che questo aiuti.

    
risposta data 06.09.2014 - 07:42
fonte
1

L'obiettivo più importante del codice è comprensibile come impegnato (in contrasto con il semplice refactoring, che è utile ma meno importante). Un esempio Python leggermente più complesso può essere usato per illustrare questo:

def value(key):
    if key == FROB:
        return FOO
    elif key == NIK:
        return BAR
    [...]
    else:
        return BAZ

Questo è abbastanza chiaro: è l'equivalente di un'istruzione case o della ricerca di un dizionario, la versione di livello superiore di return foo == bar :

KEY_VALUES = {FROB: FOO, NIK: BAR, [...]}
DEFAULT_VALUE = BAZ

def value(key):
    return KEY_VALUES.get(key, default=DEFAULT_VALUE)

Questo è più chiaro, perché anche se il numero di token è più alto (32 vs 24 per il codice originale con due coppie chiave / valore), la semantica della funzione è ora esplicita: È solo una ricerca.

(Ciò ha conseguenze per il refactoring - se vuoi avere un effetto collaterale se key == NIK , hai tre scelte:

  1. Ripristina lo stile if / elif / else e inserisci una singola riga nel caso key == NIK .
  2. Salva il risultato della ricerca su un valore e aggiungi un'istruzione if a value per il caso speciale prima di tornare.
  3. Inserisci un'istruzione if nel chiamante .

Ora vediamo perché la funzione di ricerca è una potente semplificazione: rende la terza opzione ovviamente la soluzione più semplice, poiché oltre a risultare in una differenza minore rispetto alla prima opzione è l'unica che si assicura che value fa una sola cosa. )

Tornando al codice OP, penso che questo possa servire da guida per la complessità delle dichiarazioni else : Il codice con un'istruzione else esplicita è oggettivamente più complesso, ma più importante è se rende semantica del codice chiara e semplice. E questa deve essere risolta caso per caso, poiché ogni pezzo di codice ha un significato diverso.

    
risposta data 06.09.2014 - 11:48
fonte
1

Penso che dipenda dal tipo di dichiarazione di if che stai utilizzando. Alcune dichiarazioni if possono essere considerate come espressioni e altre possono essere viste solo come istruzioni del flusso di controllo.

Il tuo esempio mi sembra un'espressione. Nei linguaggi C-like, l'operatore ternario ?: è molto simile a un'istruzione if , ma è un'espressione e la parte else non può essere omessa. Ha senso che un altro ramo non possa essere omesso in un'espressione, perché l'espressione deve sempre avere un valore.

Usando l'operatore ternario, il tuo esempio sarà simile a questo:

bool CanLeaveWithoutUmbrella()
{
    return (sky.Color == Color.Blue)
               ? true
               : false;
}

Poiché si tratta di un'espressione booleana, tuttavia, dovrebbe essere semplificata fino in fondo

bool CanLeaveWithoutUmbrella()
{
    return (sky.Color == Color.Blue);
}

Includere una clausola else esplicita rende più chiara la tua intenzione. Le guardie possono avere senso in alcune situazioni, ma nella scelta tra due valori possibili, nessuno dei quali è eccezionale o insolito, non aiutano.

    
risposta data 06.09.2014 - 12:36
fonte
1

Un'altra cosa a cui pensare in questo argomento sono le abitudini che ogni approccio promuove.

  • if / else rifatta un esempio di codifica nei termini dei rami. Come dici tu, a volte questo esplicito è buono. Ci sono davvero due possibili percorsi di esecuzione e vogliamo metterli in evidenza.

  • In altri casi, c'è solo un percorso di esecuzione e quindi tutte quelle cose eccezionali di cui i programmatori devono preoccuparsi. Come hai detto, le clausole di sicurezza sono ottime per questo.

  • Esiste, naturalmente, il caso 3 o più (n), in cui di solito incoraggiamo ad abbandonare la catena if / else e ad usare un'istruzione caso / switch o un polimorfismo.

Il principio guida che tengo a mente qui è che un metodo o una funzione dovrebbe avere un solo scopo. Qualsiasi codice con un'istruzione if / else o switch è solo per la ramificazione. Quindi dovrebbe essere assurdamente breve (4 righe) e delegare solo al metodo corretto o produrre il risultato ovvio (come il tuo esempio). Quando il tuo codice è così breve, è difficile perdere quei ritorni multipli :) Proprio come "goto considerato dannoso", qualsiasi affermazione derivata può essere abusata, quindi mettere un po 'di pensiero critico in altre affermazioni di solito vale la pena. Come hai detto, il solo blocco else fornisce un posto per il codice da raggruppare. Tu e il tuo team dovrete decidere come vi sentite.

Ciò di cui lo strumento si lamenta veramente è il fatto che hai un ritorno / interruzione / lancio all'interno del blocco if. Quindi, per quanto è preoccupato, hai scritto una clausola di guardia ma l'hai rovinato. Puoi scegliere di rendere lo strumento felice oppure puoi "intrattenere" il suo suggerimento senza accettarlo.

    
risposta data 06.09.2014 - 14:35
fonte
1

Penso che la risposta dipenda. Il tuo esempio di codice (come fai notare indirettamente) sta semplicemente restituendo la valutazione di una condizione ed è meglio rappresentato come sai, come espressione singola.

Per determinare se l'aggiunta di un'altra condizione chiarisce o oscura il codice, è necessario determinare cosa rappresenta l'IF / ELSE. È un (come nel tuo esempio) un'espressione? Se è così, quell'espressione dovrebbe essere estratta e utilizzata. È una condizione di guardia? Se è così, allora l'ELSE è inutile e fuorviante, in quanto rende i due rami appaiono equivalenti. È un esempio di polimorfismo procedurale? Estrailo. Sta impostando le condizioni preliminari? Quindi può essere essenziale.

Prima di decidere di includere o eliminare l'ELSE, decidi che cosa rappresenta, che ti dirà se dovrebbe essere presente o meno.

    
risposta data 06.09.2014 - 19:36
fonte
0

La risposta è un po 'soggettiva. Un blocco else può facilitare la leggibilità stabilendo che un blocco di codice viene eseguito esclusivamente su un altro blocco di codice, senza la necessità di leggere entrambi i blocchi. Questo può essere utile se, ad esempio, entrambi i blocchi sono relativamente lunghi. Ci sono casi, anche se dove if s senza else s può essere più leggibile. Confronta il seguente codice con un numero di blocchi if/else :

if(a == b) {
    if(c <= d) {
        if(e != f) {
            a.doSomeStuff();
            c.makeSomeThingsWith(b);
            f.undo(d, e);
            return 9;
        } else {
            e++;
            return 3;
        }
    } else {
        return 1;
    }
} else {
    return 0;
}

con:

if(a != b) {
    return 0;
}

if(c > d) {
    return 1;
}

if(e != f) {
    e++;
    return 3;
}

a.doSomeStuff();
c.makeSomeThingsWith(b);
f.undo(d, e);
return 9;

Il secondo blocco di codice modifica le dichiarazioni% co_de nidificate in clausole di guardia. Questo riduce il rientro e rende più chiare alcune condizioni di ritorno ("se if , allora restituiamo a != b !")

È stato sottolineato nei commenti che potrebbero esserci dei buchi nel mio esempio, quindi lo arricchirò un po 'di più.

Potremmo scrivere qui il primo blocco di codice in questo modo per renderlo più leggibile:

if(a != b) {
    return 0;
} else if(c > d) {
    return 1;
} else if(e != f) {
    e++;
    return 3;
} else {
    a.doSomeStuff();
    c.makeSomeThingsWith(b);
    f.undo(d, e);
    return 9;
}

Questo codice è equivalente a entrambi i blocchi sopra, ma presenta alcune sfide. La clausola 0 qui collega queste istruzioni if insieme. Nella precedente versione della clausola di salvaguardia, le clausole di guardia sono indipendenti l'una dall'altra. Aggiungerne una nuova significa semplicemente scrivere una nuova else tra l'ultima if e il resto della logica. Qui, ciò può anche essere fatto, con solo una piccola quantità di carico cognitivo maggiore. La differenza, tuttavia, è quando è necessaria qualche logica aggiuntiva prima di quella nuova clausola di guardia. Quando le dichiarazioni erano indipendenti, potremmo fare:

...
if(e != f) {
    e++;
    return 3;
}

g.doSomeLogicWith(a);

if(!g.isGood()) {
    return 4;
}

a.doSomeStuff();
...

Con la catena if collegata, questo non è così facile da fare. Di fatto, la via più facile da prendere sarebbe quella di spezzare la catena per sembrare più simile alla versione della clausola di guardia.

Ma davvero, questo ha senso. L'utilizzo di if/else implica implicitamente una relazione tra le parti. Potremmo supporre che if/else sia in qualche modo correlato a a != b nella versione c > d concatenata. Tale supposizione sarebbe fuorviante, come possiamo vedere dalla versione della clausola di salvaguardia che, in realtà, non sono correlati. Ciò significa che possiamo fare di più con clausole indipendenti, come riorganizzarle (forse per ottimizzare le prestazioni della query o migliorare il flusso logico). Possiamo anche ignorarli cognitivamente una volta che li abbiamo superati. In una catena if/else , sono meno sicuro che la relazione di equivalenza tra if/else e a non verrà ripristinata più tardi.

La relazione implicita da b è anche evidente nel codice di esempio profondamente nidificato, ma in un modo diverso. Le istruzioni else sono separate dal condizionale al quale sono associate e sono correlate nell'ordine reverse ai condizionali (il primo else è collegato all'ultimo else , l'ultimo if è collegato al primo else ). Questo crea una dissonanza cognitiva in cui dobbiamo tornare indietro nel codice, cercando di allineare indentazione nel nostro cervello. È un po 'come cercare di mettere insieme una parentesi. Diventa ancora peggio se il rientro è sbagliato. Anche le parentesi facoltative non sono di aiuto.

I forgot to mention a case in which I agree with the omission of an else block, and is when using an if block to apply a constraint that must be logically satisfied for all following code, such as a null-check (or any other guards).

Questa è una bella concessione, ma in realtà non invalida alcuna risposta. Potrei dire che nel tuo esempio di codice:

bool CanLeaveWithoutUmbrella()
{
    if(sky.Color != Color.Blue)
    {
        return false;
    }
    return true;
}

un'interpretazione valida per scrivere un codice come questo potrebbe essere che "dobbiamo partire con un ombrello solo se il cielo non è blu". Qui c'è un vincolo che il cielo deve essere blu affinché il codice seguente sia valido per essere eseguito. Ho incontrato la definizione della tua concessione.

Che cosa succede se dico che se il colore del cielo è nero, è una notte serena, quindi non è necessario alcun ombrello. (Ci sono modi più semplici per scrivere questo, ma ci sono modi più semplici per scrivere l'intero esempio.)

bool CanLeaveWithoutUmbrella()
{
    if(sky.Color == Color.Blue)
    {
        return true;
    }

    if(sky.Color == Color.Black)
    {
        return true;
    }

    return false;
}

Come nell'esempio più grande sopra, posso semplicemente aggiungere la nuova clausola senza pensarci troppo. In if , non posso essere così sicuro di non rovinare qualcosa, specialmente se la logica è più complicata.

Farò anche notare che il tuo semplice esempio non è neanche così semplice. Un test per un valore non binario non è lo stesso dell'inverso di quel test con risultati invertiti.

    
risposta data 06.09.2014 - 05:06
fonte
0

Hai semplificato il tuo esempio troppo. L'alternativa potrebbe essere più leggibile, a seconda della situazione più ampia: in particolare, dipende dal fatto che uno dei due rami della condizione sia anormale . Lascia che ti mostri: in un caso in cui entrambi i rami sono ugualmente probabili, ...

void DoOneOfTwoThings(bool which)
{
    if (which)
        DoThingOne();
    else
        DoThingTwo();
}

... è più leggibile di ...

void DoOneOfTwoThings(bool which)
{
    if (which) {
        DoThingOne();
        return;
    }
    DoThingTwo();
}

... perché il primo mostra struttura parallela e il secondo no. Ma, quando la maggior parte della funzione è dedicata a un compito e devi solo prima controllare alcune condizioni preliminari, ...

void ProcessInputOfTypeOne(String input)
{
    if (InputIsReallyTypeTwo(input)) {
        ProcessInputOfTypeTwo(input);
        return;
    }
    ProcessInputDefinitelyOfTypeOne(input);

}

... è più leggibile di ...

void ProcessInputOfTypeOne(String input)
{
    if (InputIsReallyTypeTwo(input))
        ProcessInputOfTypeTwo(input);
    else
        ProcessInputDefinitelyOfTypeOne(input);
}

... perché deliberatamente non l'impostazione della struttura parallela fornisce un indizio a un futuro lettore che ottenendo un input che è "veramente tipo due" è anormale . (Nella vita reale, ProcessInputDefinitelyOfTypeOne non sarebbe una funzione separata, quindi la differenza sarebbe ancora più netta.)

    
risposta data 06.09.2014 - 18:40
fonte
0

Sì, c'è un aumento di complessità perché la clausola else è ridondante . È quindi meglio lasciare fuori per mantenere il codice snello e cattivo. È sullo stesso accordo di omettere i qualificatori di this ridondanti (Java, C ++, C #) e omettere le parentesi nelle istruzioni return , e così via.

Un buon programmatore pensa "cosa posso aggiungere?" mentre un grande programmatore pensa "cosa posso rimuovere?".

    
risposta data 06.09.2014 - 12:21
fonte
-1

Ho notato, cambiando idea su questo caso.

Quando ho fatto questa domanda circa un anno fa, avrei risposto qualcosa del tipo:

»Ci dovrebbe essere solo un entry e un exit a un metodo« E con questo in mente, avrei suggerito di rifattorizzare il codice in:

bool CanLeaveWithoutUmbrella()
{
    bool result

    if(sky.Color == Color.Blue)
    {
        result = true;
    }
    else
    {
        result = false;
    }

    return result;
}

Da un punto di vista della comunicazione è chiaro , cosa dovrebbe essere fatto. If qualcosa è il caso, manipola il risultato in un modo , else lo manipola in un altro modo .

Ma questo comporta un inutile else . else esprime il caso predefinito . Quindi, in una seconda fase, potrei rifattenerlo in

bool CanLeaveWithoutUmbrella()
{
    bool result=false

    if(sky.Color == Color.Blue)
    {
        result = true;
    }
    return result;
}

Quindi sorge la domanda: perché usare una variabile temporanea? Il prossimo passo di refactorization porta a:

bool CanLeaveWithoutUmbrella()
{

    if(sky.Color == Color.Blue)
    {
        return true;
    }
    return false;
}

Quindi questo è chiaro in quello che fa. Vorrei anche fare un passo in più:

bool CanLeaveWithoutUmbrella()
{

    if(sky.Color == Color.Blue) return true;
    return false;
}

O per poco convinto :

bool CanLeaveWithoutUmbrella()
{

    if(sky.Color == Color.Blue) { return true; }
    return false;
}

Potresti leggerlo così: »CanLeaveWithoutUmbrella restituisce un bool . Se non accade nulla di speciale, restituisce falso «Questa è la prima lettura, dall'alto verso il basso.

E poi "analizzi" la condizione »Se la condizione è soddisfatta, restituisce true « Puoi saltare il condizionale e aver capito, cosa fa questa funzione nella default -case. Il tuo occhio e la tua mente devono solo sfogliare alcune lettere sul lato sinistro, senza leggere la parte sulla destra.

I'm under the impression the else more directly states intent, by stating the fact that the code in both blocks is directly related.

Sì, è giusto. Ma quell'informazione è ridondante . Hai un caso predefinito e una "eccezione" che è coperta da if -statement.

Quando mi occupo di strutture più complesse, sceglierei un'altra strategia, omettendo il if o il brutto fratello switch .

    
risposta data 06.09.2014 - 10:52
fonte
-1

Per farla breve, in questo caso, eliminare la parola chiave else è una buona cosa. È totalmente ridondante e, in base alla formattazione del codice, aggiungerà da uno a tre altre linee completamente inutili al codice. Considera cosa c'è nel blocco if :

return true;

Pertanto, se il blocco if dovesse essere inserito, l'intero resto della funzione è, per definizione, non eseguito. Ma se non è inserito, poiché non ci sono ulteriori dichiarazioni if , viene eseguito l'intero resto della funzione. Sarebbe totalmente diverso se un'istruzione return non fosse nel blocco if , nel qual caso la presenza o l'assenza di un blocco else avrebbe un impatto, ma qui non avrebbe alcun impatto.

Nella tua domanda, dopo aver invertito la tua condizione if e ometto else , hai dichiarato quanto segue:

Someone can now add a new if block based on a condition after the first example without immediately correctly recognizing the first condition is placing a constraint on their own condition.

Hanno bisogno di leggere il codice un po 'più da vicino allora. Utilizziamo linguaggi di alto livello e una chiara organizzazione del codice per mitigare questi problemi, ma l'aggiunta di un blocco di else completamente ridondante aggiunge "rumore" e "ingombro" al codice, e non dovremmo dover invertire o invertire nuovamente il nostro % condizioni diif. Se non possono dire la differenza, allora non sanno cosa stanno facendo. Se possono dire la differenza, possono sempre invertire le condizioni originali if .

I forgot to mention a case in which I agree with the omission of an else block, and is when using an if block to apply a constraint that must be logically satisfied for all following code, such as a null-check (or any other guards).

Questo è essenzialmente ciò che sta accadendo qui però. Stai tornando dal blocco if , in modo che la condizione if che valuta false sia un vincolo che deve essere soddisfatto logicamente per tutto il codice seguente.

Is there an increase in complexity introduced by not including the else block?

No, ciò comporterà una diminuzione della complessità del codice e potrebbe addirittura costituire una diminuzione del codice compilato, a seconda che determinate ottimizzazioni super triviali possano o meno aver luogo.

    
risposta data 06.09.2014 - 18:34
fonte
-2

Nell'esempio specifico che hai dato, lo fa.

  • Costringe un revisore a leggere di nuovo il codice per assicurarsi che non sia un errore.
  • Aggiunge (inutile) ciclomatic complessità perché aggiunge un nodo al diagramma di flusso.

Penso che non potrebbe essere più semplice questo:

bool CanLeaveWithoutUmbrella()
{
    return (sky.Color == Color.Blue);
}
    
risposta data 06.09.2014 - 04:37
fonte
-4

Cerco sempre di scrivere il mio codice in modo tale che l'intenzione sia il più chiara possibile. Il tuo esempio è privo di contesto, quindi lo modificherò un po ':

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool CanLeaveWithoutUmbrella()
    {
        if(this.color == Color.Blue)
        {
            return true;
        }
        else
        {
            return false;
        }
    }
}

La nostra intenzione sembra che possiamo lasciare senza ombrello quando Sky è blu. Tuttavia, questa semplice intenzione è persa in un mare di codice. Ci sono diversi modi in cui possiamo renderlo più facile da capire.

In primo luogo, c'è una tua domanda sul ramo else . Non mi dispiace in entrambi i casi, ma tendono a lasciare fuori il else . Capisco l'argomento sull'essere espliciti, ma la mia opinione è che le dichiarazioni di if dovrebbero racchiudere i rami 'facoltativi', come return true uno. Non chiamerei "% facoltativo" il ramo return false , poiché agisce come predefinito. Ad ogni modo, vedo questo come un problema molto minore, e molto meno importante dei seguenti:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool CanLeaveWithoutUmbrella()
    {
        if(this.color == Color.Blue)
        {
            return true;
        }
        return false;
    }
}

Un problema molto più importante è che i tuoi rami stanno facendo troppo! I rami, come tutto, dovrebbero essere "il più semplice possibile, ma non di più". In questo caso hai utilizzato un'istruzione if , che si dirama tra blocchi di codice (raccolte di istruzioni) quando tutto ciò che ti serve è una diramazione tra le espressioni (valori) . Questo è chiaro dal momento che a) i blocchi di codice contengono solo dichiarazioni singole e b) sono la stessa istruzione ( return ). Possiamo usare l'operatore ternario ?: per restringere il ramo alla sola parte diversa:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool CanLeaveWithoutUmbrella()
    {
        return (this.color == Color.Blue)
            ? true
            : false;
    }
}

Ora raggiungiamo l'evidente ridondanza che il nostro risultato è identico a this.color == Color.Blue . So che la tua domanda ci chiede di ignorarlo, ma penso che sia davvero importante sottolineare che questo è un modo di pensare procedurale di prim'ordine: stai analizzando i valori di input e costruendo alcuni valori di output. Va bene, ma se possibile è molto più potente pensare in termini di relazioni o trasformazioni tra l'input e l'output. In questo caso, la relazione è evidente che sono uguali, ma spesso vedo un codice procedurale contorto come questo, che oscura alcune semplici relazioni. Andiamo avanti e facciamo questa semplificazione:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool CanLeaveWithoutUmbrella()
    {
        return (this.color == Color.Blue);
    }
}

La prossima cosa che vorrei sottolineare è che la tua API contiene un doppio negativo: è possibile che CanLeaveWithoutUmbrella sia false . Questo, ovviamente, semplifica fino a NeedUmbrella come true , quindi facciamolo:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool NeedUmbrella()
    {
        return (this.color != Color.Blue);
    }
}

Ora possiamo iniziare a pensare al tuo stile di codifica. Sembra che tu stia usando un linguaggio orientato agli oggetti, ma usando le istruzioni di if per diramarti tra i blocchi di codice, hai finito per scrivere codice procedurale .

Nel codice orientato agli oggetti, tutti i blocchi di codice sono metodi e tutte le diramazioni vengono eseguite tramite dispatch dinamico , ad es. usando classi o prototipi. È discutibile se ciò rende le cose più semplici, ma dovrebbe rendere il tuo codice più coerente con il resto della lingua:

class Sky {
    bool NeedUmbrella()
    {
        return true;
    }
}

class BlueSky extends Sky {
    bool NeedUmbrella()
    {
        return false;
    }
}

Si noti che l'uso di operatori ternari per diramare le espressioni è comune nella programmazione funzionale .

    
risposta data 06.09.2014 - 16:43
fonte