Inversione di un'istruzione IF

Dipende. Nel tuo esempio l'istruzione non invertita if non è un problema, perché il corpo di if è molto breve. Tuttavia di solito vuoi invertire le affermazioni quando i corpi crescono.

Siamo solo umani e mantenere più cose alla volta nella nostra testa è difficile.

Quando il corpo di un'istruzione è grande, l'inversione dell'istruzione non solo riduce il nesting, ma dice allo sviluppatore che possono dimenticare tutto ciò che si trova al di sopra di tale affermazione e concentrarsi solo sul resto. È molto probabile che tu usi le istruzioni invertite per sbarazzarti dei valori sbagliati il prima possibile. Una volta che sai che quelli sono fuori dal gioco, l'unica cosa che rimane sono i valori che possono essere effettivamente elaborati.

Non evitare i negativi. Gli umani succhiano mentre li analizzano anche quando la CPU li adora.

Tuttavia, rispetta gli idiomi. Noi umani siamo creature abitudinarie, quindi preferiamo percorsi ben usurati per dirigere percorsi pieni di erbe infestanti.

foo != null è diventato, purtroppo, un idioma. Non noto più le parti separate. Lo guardo e penso, "oh, è sicuro puntare su foo ora".

Se vuoi ribaltare questo (oh, un giorno, per favore) hai bisogno di un caso davvero strong. Hai bisogno di qualcosa che lasci che i miei occhi scivolino via da esso e comprendilo in un istante.

Tuttavia, quello che ci hai dato era questo:

foreach (someObject in someObjectList)
{
    if(someObject == null) continue;
    someOtherObject = someObject.SomeProperty;
}

Che non è nemmeno strutturalmente uguale!

foreach (someObject in someObjectList)
{
    if(someObject == null) continue;
    someOtherObject = someObject.SomeProperty;

    if(someGlobalObject == null) continue;
    someSideEffectObject = someGlobalObject.SomeProperty;
}

Questo non sta facendo quello che il mio cervello pigro si aspettava che facesse. Pensavo di poter continuare ad aggiungere codice indipendente, ma non posso. Questo mi fa pensare a cose a cui non voglio pensare ora. Hai rotto il mio idioma ma non me ne hai dato uno migliore. Tutto perché volevi proteggermi dall'unico negatore che ha bruciato il suo piccolo brutto io nella mia anima.

Quindi mi dispiace, forse ReSharper ha ragione a suggerire questo 90% delle volte, ma nei controlli nulli penso che tu abbia trovato un caso d'angolo in cui è meglio ignorarlo. Gli strumenti semplicemente non sono bravi a insegnarti quale codice leggibile è. Chiedi a un umano. Fai una revisione tra pari. Ma non seguire ciecamente lo strumento.

È la vecchia discussione sul fatto che un'interruzione sia peggiore di quella di nidificazione.

Le persone sembrano accettare il ritorno anticipato per i controlli dei parametri, ma è sfigurato nella maggior parte di un metodo.

Personalmente evito di continuare, interrompere, andare ecc. anche se ciò significa più nidificazione. Ma nella maggior parte dei casi puoi evitare entrambi.

foreach (someObject in someObjectList.where(i=>i != null))
{
    someOtherObject = someObject.SomeProperty;
}

Ovviamente il nesting rende le cose difficili da seguire quando hai molti livelli

foreach (someObject in someObjectList) 
{
    if(someObject != null) 
    {
        someOtherObject = someObject.SomeProperty;
        if(someObject.x > 5) 
        {
            x ++;
            if(someObject.y > 5) 
            {
                x ++;
            }
        }
    }
}

Il peggio è quando hai entrambi, però

foreach (someObject in someObjectList) 
{
    if(someObject != null) 
    {
        someOtherObject = someObject.SomeProperty;
        if(someObject.x > 5) 
        {
            x ++;
            if(someObject.y > 5) 
            {
                x ++;
                return;
            }
            continue;
        }
        someObject.z --;
        if(myFunctionWithSideEffects(someObject) > 6) break;
    }
}

Il problema con continue è che se aggiungi più linee al codice la logica diventa complicata e probabilmente contiene bug. per es.

foreach (someObject in someObjectList)
{
    if(someObject == null) continue;
    someOtherObject = someObject.SomeProperty;

    ...  imagine that there is some code here ...

    // added later by a Junior Developer
    doSomeOtherFunction(someObject);
}

Vuoi chiamare doSomeOtherFunction() per tutti someObject, o solo se non-null? Con il codice originale hai l'opzione ed è più chiaro. Con continue si potrebbe dimenticare "oh, sì, lì dietro abbiamo ignorato i null" e non hai nemmeno la possibilità di chiamarlo per tutti gli oggetti, a meno che tu non metta quella chiamata all'inizio del metodo che potrebbe non essere possibile o corretto per altri motivi.

Sì, un sacco di nidificazione è brutto e forse confuso, ma in questo caso userei lo stile tradizionale.

Domanda bonus: Perché ci sono valori nulli in quell'elenco per cominciare? Potrebbe essere preferibile non aggiungere mai un valore null in primo luogo, nel qual caso la logica di elaborazione è molto più semplice.

Questa tecnica è utile per certificare le pre-condizioni quando la maggior parte del tuo metodo si verifica quando tali condizioni sono soddisfatte.

Spesso invertiamo ifs sul mio lavoro e impieghiamo un altro fantasma. Era piuttosto frequente che durante la revisione di una PR potrei indicare come il mio collega potrebbe rimuovere tre livelli di nidificazione! Succede meno frequentemente da quando pubblichiamo i nostri if.

Ecco un esempio di giocattolo che può essere srotolato

function foobar(x, y, z) {
    if (x > 0) {
        if (z != null && isGamma(y)) {
            // ~10 lines of code
            // return something
        }
        else {
           return y
        }
    }
    else {
      return y + z
    }
}

Codice come questo, dove c'è UN caso interessante (dove il resto sono semplicemente resi o piccoli blocchi di istruzioni) sono comuni. È banale riscrivere quanto sopra come

function foobar(x, y, z) {
    if (x <=0) {
        return y + z
    }
    if (z == null || !isGamma(y) {
       return y
    }
    // ~ 10 lines of code
    // return something
}

Qui, il nostro codice significativo, le 10 righe non incluse, non ha il triplo indentato nella funzione. Se hai il primo stile, sei tentato di rifattorizzare il blocco lungo in un metodo o di tollerare il rientro. Qui è dove si accumulano i risparmi. In un posto questo è un risparmio banale ma attraverso molti metodi in molte classi, risparmi la necessità di generare una funzione extra per rendere il codice più pulito e non hai così orribile rientro multi-livello .

In risposta all'esempio del codice OP, concordo sul fatto che si tratta di un collasso eccessivamente oneroso. Soprattutto perché in 1 TB, lo stile che uso, l'inversione fa aumentare il codice di dimensioni.

L'idea è il ritorno anticipato. All'inizio return in un ciclo diventa presto continue .

Un sacco di buone risposte su questa domanda: Devo tornare da una funzione in anticipo o usare un'istruzione if?

Si noti che mentre la domanda è chiusa in base all'opinione, 430+ voti sono a favore del ritorno anticipato, ~ 50 voti sono "dipende" e 8 sono contro il ritorno anticipato. Quindi c'è un consenso eccezionalmente strong (o quello, o sono cattivo nel conteggio, che è plausibile).

Personalmente, se il corpo del ciclo sarebbe soggetto a continuare in anticipo e abbastanza a lungo per farlo (3-4 righe), tendo ad estrarre il corpo del ciclo in una funzione in cui utilizzo il ritorno anticipato invece di continuare presto.

I suggerimenti dei resharpers sono spesso utili ma dovrebbero sempre essere valutati in modo critico. Cerca alcuni modelli di codice predefiniti e suggerisce le trasformazioni, ma non può tenere conto di tutti i fattori che rendono il codice più o meno leggibile per gli esseri umani.

A parità di condizioni, è preferibile una nidificazione minore, motivo per cui ReSharper suggerisce una trasformazione che riduce il nesting. Ma tutte le altre cose sono non uguali: in questo caso la trasformazione richiede l'introduzione di un continue , il che significa che il codice risultante è in realtà più difficile da seguire.

In alcuni casi, lo scambio di un livello di nidificazione per un continue potrebbe essere effettivamente un miglioramento, ma ciò dipende dalla semantica del codice in questione, che è oltre la comprensione delle regole meccaniche di ReSharpers.

Nel tuo caso il suggerimento di ReSharper potrebbe peggiorare il codice. Quindi ignoralo. O meglio: non utilizzare la trasformazione suggerita, ma dare un po 'di pensiero su come il codice potrebbe essere migliorato. Si noti che è possibile assegnare la stessa variabile più volte, ma solo l'ultimo compito ha effetto, poiché il precedente sarebbe stato semplicemente sovrascritto. Questo fa sembra un insetto. Il suggerimento di ReSharpers non risolve il bug, ma rende un po 'più ovvio che sta accadendo qualche logica dubbiosa.

Penso che il punto più grande qui sia che lo scopo del loop determina il modo migliore per organizzare il flusso all'interno del ciclo. Se stai filtrando alcuni elementi prima di scorrere il rimanente, allora continue -ing out ha senso. Tuttavia, se stai facendo diversi tipi di elaborazione all'interno di un ciclo, potrebbe essere più sensato rendere quel if davvero ovvio, come ad esempio:

for(obj in objectList) {
    if(obj == null) continue;
    if(obj.getColour().equals(Color.BLUE)) {
        // Handle blue objects
    } else {
        // Handle non-blue objects
    }
}

Si noti che in Stream Java o altri idiomi funzionali, è possibile separare il filtraggio dal loop, utilizzando:

items.stream()
    .filter(i -> (i != null))
    .filter(i -> i.getColour().equals(Color.BLUE))
    .forEach(i -> {
        // Process blue objects
    });

Per inciso, questa è una delle cose che amo del inverted if di Perl ( unless ) applicato a singole istruzioni, che consente il seguente tipo di codice, che trovo molto facile da leggere:

foreach my $item (@items) {
    next unless defined $item;
    carp "Only blue items are supported! Failed on item $item" 
       unless ($item->get_colour() eq 'blue');
    ...
}