Invertire l'istruzione "if" per ridurre l'annidamento

https://stackoverflow.com/questions/268132

c#
resharper

06-07-2019
|

Domanda

Quando ho eseguito ReSharper sul mio codice, ad esempio:

    if (some condition)
    {
        Some code...            
    }

ReSharper mi ha dato l'avviso di cui sopra (Inverti istruzione "if" per ridurre l'annidamento) e ha suggerito la seguente correzione:

   if (!some condition) return;
   Some code...

Vorrei capire perché è meglio. Ho sempre pensato che usando " return " nel mezzo di un metodo problematico, un po 'come "vai a"

Soluzione

Un ritorno nel mezzo del metodo non è necessariamente negativo. Potrebbe essere meglio restituire immediatamente se rende più chiaro l'intento del codice. Ad esempio:

double getPayAmount() {
    double result;
    if (_isDead) result = deadAmount();
    else {
        if (_isSeparated) result = separatedAmount();
        else {
            if (_isRetired) result = retiredAmount();
            else result = normalPayAmount();
        };
    }
     return result;
};

In questo caso, se _isDead è vero, possiamo immediatamente uscire dal metodo. Potrebbe essere meglio strutturarlo in questo modo invece:

double getPayAmount() {
    if (_isDead)      return deadAmount();
    if (_isSeparated) return separatedAmount();
    if (_isRetired)   return retiredAmount();

    return normalPayAmount();
};

Ho scelto questo codice dal catalogo di refactoring . Questo refactoring specifico è chiamato: Sostituisci condizionale nidificato con clausole di guardia.

Altri suggerimenti

Non è solo estetico , ma riduce anche livello massimo di annidamento all'interno del metodo. Questo è generalmente considerato un vantaggio perché semplifica la comprensione dei metodi (e in effetti molti static analisi strumenti fornisce una misura di questo come uno degli indicatori della qualità del codice).

D'altra parte, fa anche in modo che il tuo metodo abbia più punti di uscita, qualcosa che un altro gruppo di persone crede sia un no-no.

Personalmente, sono d'accordo con ReSharper e il primo gruppo (in una lingua che ha delle eccezioni trovo sciocco discutere di "punti di uscita multipli"; quasi tutto può essere lanciato, quindi ci sono numerosi potenziali punti di uscita in tutti i metodi).

Riguardo alle prestazioni : entrambe le versioni dovrebbero essere equivalenti (se non a livello di IL, quindi sicuramente dopo che il jitter è finito con il codice) in ogni lingua. Teoricamente questo dipende dal compilatore, ma praticamente qualsiasi compilatore ampiamente utilizzato di oggi è in grado di gestire casi molto più avanzati di ottimizzazione del codice di questo.

Questo è un po 'un argomento religioso, ma concordo con ReSharper che dovresti preferire meno nidificazione. Credo che questo superi i negativi di avere più percorsi di ritorno da una funzione.

Il motivo principale per avere meno nidificazione è migliorare leggibilità e manutenibilità del codice . Ricorda che molti altri sviluppatori dovranno leggere il tuo codice in futuro e che il codice con meno rientri è generalmente molto più facile da leggere.

Presupposti sono un ottimo esempio di dove è possibile tornare presto all'inizio della funzione. Perché la leggibilità del resto della funzione dovrebbe essere influenzata dalla presenza di un controllo preliminare?

Per quanto riguarda gli aspetti negativi relativi al ritorno più volte da un metodo, i debugger sono piuttosto potenti ora, ed è molto facile scoprire esattamente dove e quando sta tornando una particolare funzione.

Avere più ritorni in una funzione non influirà sul lavoro del programmatore di manutenzione.

Scarsa leggibilità del codice.

Come altri hanno già detto, non ci dovrebbe essere un successo nelle prestazioni, ma ci sono altre considerazioni. A parte queste valide preoccupazioni, questo può anche aprirti ai gotchas in alcune circostanze. Supponiamo che tu abbia a che fare con un double invece:

public void myfunction(double exampleParam){
    if(exampleParam > 0){
        //Body will *not* be executed if Double.IsNan(exampleParam)
    }
}

Contrasta quello con l'inversione apparentemente equivalente:

public void myfunction(double exampleParam){
    if(exampleParam <= 0)
        return;
    //Body *will* be executed if Double.IsNan(exampleParam)
}

Quindi in determinate circostanze quello che sembra essere un correttamente invertito se potrebbe non esserlo.

L'idea di tornare solo alla fine di una funzione è tornata dai giorni prima che le lingue avessero il supporto per le eccezioni. Ha permesso ai programmi di fare affidamento sul fatto di poter mettere il codice di pulizia alla fine di un metodo, e quindi essere sicuro che sarebbe stato chiamato e qualche altro programmatore non avrebbe nascosto un ritorno nel metodo che ha causato il salto del codice di pulizia . Il codice di pulizia ignorato potrebbe provocare una perdita di memoria o di risorse.

Tuttavia, in una lingua che supporta le eccezioni, non fornisce tali garanzie. In un linguaggio che supporta le eccezioni, l'esecuzione di qualsiasi istruzione o espressione può causare un flusso di controllo che provoca la fine del metodo. Ciò significa che la pulizia deve essere eseguita utilizzando il metodo finally o utilizzando le parole chiave.

In ogni caso, sto dicendo che penso che molte persone citino la linea guida "solo ritorno alla fine di un metodo" senza capire perché sia ??mai stata una buona cosa da fare e che ridurre la nidificazione per migliorare la leggibilità è probabilmente un obiettivo migliore.

Vorrei aggiungere che esiste un nome per gli if invertiti: clausola di guardia. Lo uso ogni volta che posso.

Odio leggere il codice dove c'è se all'inizio, due schermate di codice e nessun altro. Basta invertire se e tornare. In questo modo nessuno perderà tempo a scorrere.

http://c2.com/cgi/wiki?GuardClause

Non influisce solo sull'estetica, ma impedisce anche l'annidamento del codice.

Può effettivamente funzionare come condizione preliminare per garantire che anche i tuoi dati siano validi.

Questo è ovviamente soggettivo, ma penso che migliora notevolmente su due punti:

Ora è immediatamente ovvio che la tua funzione non ha più nulla da fare se la condition è valida.
Mantiene basso il livello di annidamento. La nidificazione danneggia la leggibilità più di quanto si pensi.

Più punti di ritorno sono stati un problema in C (e in misura minore C ++) perché ti hanno costretto a duplicare il codice di pulizia prima di ciascuno dei punti di ritorno. Con Garbage Collection, il prova | finalmente costruisce e usando i blocchi, non c'è davvero alcun motivo per cui dovresti aver paura di loro.

Alla fine si riduce a ciò che tu e i tuoi colleghi trovate più facile da leggere.

Per quanto riguarda le prestazioni, non vi sarà alcuna differenza evidente tra i due approcci.

Ma la codifica non riguarda solo le prestazioni. Anche la chiarezza e la manutenibilità sono molto importanti. E, in casi come questo in cui non influisce sulle prestazioni, è l'unica cosa che conta.

Esistono scuole di pensiero in competizione su quale approccio sia preferibile.

Una vista è quella che altri hanno menzionato: il secondo approccio riduce il livello di annidamento, migliorando la chiarezza del codice. Questo è naturale in uno stile imperativo: quando non hai più nulla da fare, potresti anche tornare presto.

Un altro punto di vista, dal punto di vista di uno stile più funzionale, è che un metodo dovrebbe avere un solo punto di uscita. Tutto in un linguaggio funzionale è un'espressione. Quindi, se le dichiarazioni devono sempre avere clausole else. Altrimenti l'espressione if non avrebbe sempre un valore. Quindi, nello stile funzionale, il primo approccio è più naturale.

Le clausole di protezione o le pre-condizioni (come probabilmente vedrai) verificano se una determinata condizione è soddisfatta e quindi interrompe il flusso del programma. Sono perfetti per i luoghi in cui sei davvero interessato solo a un risultato di un'istruzione if . Quindi, piuttosto che dire:

if (something) {
    // a lot of indented code
}

Si inverte la condizione e si interrompe se tale condizione invertita è soddisfatta

if (!something) return false; // or another value to show your other code the function did not execute

// all the code from before, save a lot of tabs

return non è affatto sporco come goto . Ti consente di passare un valore per mostrare al resto del codice che la funzione non può essere eseguita.

Vedrai i migliori esempi di dove questo può essere applicato in condizioni nidificate:

if (something) {
    do-something();
    if (something-else) {
        do-another-thing();
    } else {
        do-something-else();
    }
}

if (!something) return;
do-something();

if (!something-else) return do-something-else();
do-another-thing();

Troverai poche persone che sostengono che il primo è più pulito, ma ovviamente è completamente soggettivo. Ad alcuni programmatori piace sapere a quali condizioni funziona qualcosa per rientro, mentre preferirei mantenere il flusso del metodo lineare.

Non suggerirò per un momento che i precons cambieranno la tua vita o ti faranno scopare, ma potresti trovare il tuo codice un po 'più facile da leggere.

Qui ci sono molti buoni punti, ma anche più punti di ritorno possono essere illeggibili , se il metodo è molto lungo. Detto questo, se hai intenzione di utilizzare più punti di ritorno assicurati solo che il tuo metodo sia breve, altrimenti il ??bonus di leggibilità di più punti di ritorno potrebbe andare perso.

Le prestazioni sono suddivise in due parti. Hai prestazioni quando il software è in produzione, ma vuoi anche avere prestazioni durante lo sviluppo e il debug. L'ultima cosa che uno sviluppatore desidera è "attendere" per qualcosa di banale. Alla fine, compilando questo con l'ottimizzazione abilitata si otterrà un codice simile. Quindi è bello conoscere questi piccoli trucchi che ripagano in entrambi gli scenari.

Il caso nella domanda è chiaro, ReSharper è corretto. Invece di annidare le istruzioni if e creare un nuovo ambito nel codice, stai impostando una regola chiara all'inizio del metodo. Aumenta la leggibilità, sarà più facile da mantenere e riduce la quantità di regole che bisogna esaminare per trovare dove vogliono andare.

Personalmente preferisco solo 1 punto di uscita. È facile da realizzare se mantieni i tuoi metodi brevi e mirati e fornisce un modello prevedibile per la persona successiva che lavora sul tuo codice.

ad es.

 bool PerformDefaultOperation()
 {
      bool succeeded = false;

      DataStructure defaultParameters;
      if ((defaultParameters = this.GetApplicationDefaults()) != null)
      {
           succeeded = this.DoSomething(defaultParameters);
      }

      return succeeded;
 }

Questo è anche molto utile se vuoi solo controllare i valori di alcune variabili locali all'interno di una funzione prima che esca. Tutto quello che devi fare è posizionare un breakpoint sul ritorno finale e sei sicuro di colpirlo (a meno che non venga generata un'eccezione).

Molte buone ragioni per come appare il codice . Ma che dire dei risultati ?

Diamo un'occhiata al codice C # e al suo modulo compilato IL:


using System;

public class Test {
    public static void Main(string[] args) {
        if (args.Length == 0) return;
        if ((args.Length+2)/3 == 5) return;
        Console.WriteLine("hey!!!");
    }
}

Questo semplice frammento può essere compilato. È possibile aprire il file .exe generato con ildasm e verificare qual è il risultato. Non posterò tutte le cose dell'assemblatore ma descriverò i risultati.

Il codice IL generato genera quanto segue:

Se la prima condizione è falsa, passa al codice in cui si trova la seconda.
Se è vero salta all'ultima istruzione. (Nota: l'ultima istruzione è un ritorno).
Nella seconda condizione lo stesso accade dopo che il risultato è stato calcolato. Confronta e: arriva a Console.WriteLine se falso o alla fine se questo è vero.
Stampa il messaggio e ritorna.

Quindi sembra che il codice salterà alla fine. E se facciamo un normale se con codice nidificato?

using System;

public class Test {
    public static void Main(string[] args) {
        if (args.Length != 0 && (args.Length+2)/3 != 5) 
        {
            Console.WriteLine("hey!!!");
        }
    }
}

I risultati sono abbastanza simili nelle istruzioni IL. La differenza è che prima c'erano dei salti per condizione: se falso vai al prossimo pezzo di codice, se vero vai alla fine. E ora il codice IL scorre meglio e ha 3 salti (il compilatore ha ottimizzato un po 'questo): 1. Primo salto: quando Lunghezza è da 0 a una parte in cui il codice salta di nuovo (Terzo salto) fino alla fine. 2. Secondo: nel mezzo della seconda condizione per evitare un'istruzione. 3. Terzo: se la seconda condizione è falsa, vai alla fine.

Comunque, il contatore del programma salterà sempre.

Questo è semplicemente controverso. Non esiste un "accordo tra i programmatori" sulla questione del ritorno anticipato. È sempre soggettivo, per quanto ne so.

È possibile presentare un argomento di prestazione, poiché è meglio avere condizioni scritte in modo che siano spesso vere; si può anche sostenere che è più chiaro. D'altra parte, crea test nidificati.

Non credo che otterrai una risposta definitiva a questa domanda.

In teoria, l'inversione di if potrebbe portare a prestazioni migliori se aumenta la percentuale di hit di previsione del ramo. In pratica, penso che sia molto difficile sapere esattamente come si comporteranno le previsioni del ramo, specialmente dopo la compilazione, quindi non lo farei nel mio sviluppo quotidiano, tranne se scrivo il codice assembly.

Altre informazioni sulla previsione delle filiali qui .

Esistono già molte risposte perspicaci, ma vorrei comunque indirizzarmi verso una situazione leggermente diversa: invece di un presupposto, che dovrebbe essere messo al primo posto di una funzione, pensa a un'inizializzazione passo-passo , in cui è necessario verificare che ogni passaggio abbia esito positivo e quindi passare al successivo. In questo caso, non puoi controllare tutto in alto.

Ho trovato il mio codice davvero illeggibile quando ho scritto un'applicazione host ASIO con l'ASIOSDK di Steinberg, mentre seguivo il paradigma del nesting. Sono passati otto livelli di profondità e non riesco a vedere un difetto di progettazione lì, come menzionato da Andrew Bullock sopra. Ovviamente, avrei potuto impacchettare del codice interno in un'altra funzione e quindi annidare i livelli rimanenti lì per renderlo più leggibile, ma questo mi sembra piuttosto casuale.

Sostituendo l'annidamento con clausole di guardia, ho anche scoperto un mio malinteso su una porzione di codice di pulizia che avrebbe dovuto verificarsi molto prima all'interno della funzione anziché alla fine. Con i rami nidificati, non l'avrei mai visto, potresti anche dire che hanno portato al mio malinteso.

Quindi questa potrebbe essere un'altra situazione in cui ifs invertiti possono contribuire a un codice più chiaro.

Evitare punti di uscita multipli può portare a miglioramenti delle prestazioni. Non sono sicuro di C # ma in C ++ l'ottimizzazione del valore restituito con nome (Copy Elision, ISO C ++ '03 12.8 / 15) dipende dall'avere un singolo punto di uscita. Questa ottimizzazione evita che la copia costruisca il valore restituito (nell'esempio specifico non importa). Ciò potrebbe portare a notevoli miglioramenti delle prestazioni in cicli stretti, poiché si salva un costruttore e un distruttore ogni volta che viene invocata la funzione.

Ma per il 99% dei casi il salvataggio delle chiamate aggiuntive di costruttore e distruttore non vale la perdita della leggibilità nidificata se vengono introdotti blocchi (come altri hanno sottolineato).

È una questione di opinione.

Il mio approccio normale sarebbe quello di evitare ifs a riga singola e ritorna nel mezzo di un metodo.

Non vorresti linee come suggerisce ovunque nel tuo metodo, ma c'è qualcosa da dire per controllare un mucchio di ipotesi nella parte superiore del tuo metodo e fare il tuo vero lavoro solo se tutti passano.

A mio avviso, il ritorno anticipato va bene se stai solo restituendo il vuoto (o qualche codice di ritorno inutile che non controllerai mai) e potrebbe migliorare la leggibilità perché eviti l'annidamento e allo stesso tempo rendi esplicito che la tua funzione è fatto.

Se stai effettivamente restituendo returnValue - l'annidamento è di solito un modo migliore per andare perché restituisci il returnValue solo in un posto (alla fine - duh) e potrebbe rendere il tuo codice più gestibile in molti casi .

Penso che dipenda da ciò che preferisci, come detto, non c'è nessun accordo generale dopo. Per ridurre il fastidio, puoi ridurre questo tipo di avviso a "Suggerimento"

La mia idea è che il ritorno "nel mezzo di una funzione" non dovrebbe essere così "soggettivo". Il motivo è abbastanza semplice, prendi questo codice:

    function do_something( data ){

      if (!is_valid_data( data )) 
            return false;


       do_something_that_take_an_hour( data );

       istance = new object_with_very_painful_constructor( data );

          if ( istance is not valid ) {
               error_message( );
                return ;

          }
       connect_to_database ( );
       get_some_other_data( );
       return;
    }

Forse il primo " return " non è COSÌ intuitivo, ma è davvero salvifico. Ci sono troppe " idee " sui codici puliti, che hanno semplicemente bisogno di più pratica per perdere il loro "soggettivo" cattive idee.

Ci sono molti vantaggi in questo tipo di codifica, ma per me la grande vittoria è che se riesci a tornare rapidamente puoi migliorare la velocità della tua applicazione. IE So che grazie alla Precondizione X posso tornare rapidamente con un errore. Questo elimina prima i casi di errore e riduce la complessità del codice. In molti casi, poiché ora la pipeline della cpu può essere più pulita, può arrestare gli arresti o gli interruzioni della pipeline. In secondo luogo, se sei in loop, interrompere o tornare rapidamente può farti risparmiare un sacco di CPU. Alcuni programmatori utilizzano invarianti di loop per eseguire questo tipo di uscita rapida, ma in questo caso è possibile interrompere la pipeline della cpu e persino creare problemi di ricerca della memoria e indicare che la cpu deve caricare dalla cache esterna. Ma fondamentalmente penso che dovresti fare quello che volevi, cioè terminare il ciclo o la funzione non creare un percorso di codice complesso solo per implementare una nozione astratta di codice corretto. Se l'unico strumento che hai è un martello, allora tutto sembra un chiodo.

Non sono sicuro, ma penso che R # cerchi di evitare salti lontani. Quando hai IF-ELSE, il compilatore fa qualcosa del genere:

Condizione false - > salto in lungo verso false_condition_label

true_condition_label: Istruzione1 ... instruction_n

false_condition_label: Istruzione1 ... instruction_n

blocco finale

Se la condizione è vera, non vi è alcun salto e nessuna implementazione della cache L1, ma il salto a false_condition_label può essere molto lontano e il processore deve implementare la propria cache. La sincronizzazione della cache è costosa. R # prova a sostituire i salti lontani con i salti brevi e in questo caso vi sono maggiori probabilità che tutte le istruzioni siano già nella cache.

Autorizzato sotto: CC-BY-SA insieme a attribuzione

Non affiliato a StackOverflow