Come si dovrebbe rifattorizzare le dichiarazioni IF annidate? [duplicare]

È davvero difficile dirlo senza sapere come interagiscono i diversi controlli. Un rigoroso refactoring potrebbe essere in ordine. Creando una topologia di oggetti che eseguono il blocco corretto in base al loro tipo, anche un modello di strategia o uno schema di stato potrebbe fare il trucco.

Senza sapere cosa fare meglio, prenderei in considerazione due possibili semplici refactoring che potrebbero essere ulteriormente refactored estraendo più metodi.

Il primo non mi piace molto visto che mi piacciono sempre come punti di uscita in un metodo (preferibilmente uno)

if (!Check#1)
{ 
    return;
}
CodeBlock#1

if (!Check#2)
{
    return;
}
CodeBlock#2
...

Il secondo rimuove i ritorni multipli ma aggiunge anche molto rumore. (in pratica rimuove solo il nesting)

bool stillValid = Check#1
if (stillValid)
{
  CodeBlock#1
}

stillValid = stillValid && Check#2
if (stillValid)
{
  CodeBlock#2
}

stillValid = stillValid && Check#3
if (stillValid)
{
  CodeBlock#3
}
...

Quest'ultimo può essere riadattato piacevolmente nelle funzioni e quando dai loro dei buoni nomi il risultato potrebbe essere ragionevole ';

bool stillValid = DoCheck1AndCodeBlock1()
stillValid = stillValid && DoCheck2AndCodeBlock2()
stillValid = stillValid && DoCheck3AndCodeBlock3()

public bool DoCheck1AndCodeBlock1()
{
   bool returnValid = Check#1
   if (returnValid)
   {
      CodeBlock#1
   }
   return returnValid
}

Tutto sommato ci sono molto probabilmente le migliori opzioni

Si chiama "Arrow Code" a causa della forma del codice con il rientro appropriato.

Jeff Atwood ha avuto un buon post sul blog su Coding Horror su come appiattire le frecce:

Codice freccia di appiattimento

Leggi l'articolo per il trattamento completo, ma qui ci sono i punti principali ..

• Sostituisci le condizioni con clausole di guardia
• Decompone i blocchi condizionali in funzioni separate
• Converti assegni negativi in assegni positivi
• Restituisci sempre opportunisticamente il prima possibile dalla funzione

So che alcune persone sostengono che è un goto, ma return; è l'ovvio refactoring, cioè

if (!Check#1)
{ 
        return;
}
CodeBlock#1
if (!Check#2)
{
    return;
}
CodeBlock#2
.
.
.
if (Check#7)
{
    CodeBlock#7
}
else
{
    rest - of - the - program
}

Se in realtà è solo una serie di controlli di guardia prima di eseguire il codice, allora funziona bene. Se è più complicato di così, allora sarà solo un po 'più semplice e avrai bisogno di una delle altre soluzioni.

Il codice spaghetti sembra il candidato perfetto per refactoring in una macchina a stati.

Questo potrebbe essere già stato menzionato, ma il mio "go-to" (gioco di parole) risposta alla "punta di freccia antipattern" qui mostrata è per invertire i "se". Prova il contrario delle condizioni attuali (abbastanza facile con un operatore non) e torna dal metodo se è vero. Niente gotos (anche se pedanticamente parlando, un ritorno vuoto è poco più di un salto alla linea del codice chiamante, con il banale passaggio extra di far scoppiare una trama fuori dallo stack).

Caso in questione, ecco l'originale:

if (Check#1)
{
    CodeBlock#1
    if (Check#2)
    {
        CodeBlock#2
        if (Check#3)
        {
            CodeBlock#3
            if (Check#4)
            {
                CodeBlock#4
                if (Check#5)
                {
                    CodeBlock#5
                    if (Check#6)
                    {
                        CodeBlock#6
                        if (Check#7)
                        {
                            CodeBlock#7
                        }
                        else
                        {
                            rest - of - the - program
                        }
                    }
                }
            }
        }
    }
}

refactoring:

if (!Check#1) return;

CodeBlock#1

if (!Check#2) return;

CodeBlock#2

if (!Check#3) return;

CodeBlock#3

if (!Check#4) return;

CodeBlock#4

if (!Check#5) return;

CodeBlock#5

if (!Check#6) return;

CodeBlock#6

if (Check#7)
    CodeBlock#7
else
{
    //rest of the program
}

A ciascuno se, fondamentalmente controlliamo per vedere se dovremmo continuare. Funziona esattamente allo stesso modo dell'originale con un solo livello di nidificazione.

Se c'è qualcosa oltre la fine di questo snippet che dovrebbe essere eseguito, estrai questo codice nel suo metodo e chiamalo da dove questo codice attualmente risiede, prima di procedere al codice che verrebbe dopo questo snippet. Lo snippet stesso è abbastanza lungo, dato un LOC effettivo sufficiente in ogni blocco di codice, per giustificare la suddivisione di molti più metodi, ma sto divagando.

Utilizzando un approccio OO a un modello composito in cui la foglia è una condizione e un componente semplici, l'unione di elementi semplici rende questo tipo di codice estendibile e adattabile

Se hai regolarmente la logica che richiede effettivamente questa piramide di controlli if , probabilmente stai usando (in modo metaforico) una chiave inglese per martellare le unghie. Sarai servito meglio a fare questo tipo di logica complicata e distorta in un linguaggio che supporta quel tipo di logica complicata e distorta con strutture migliori rispetto ai costrutti in stile lineare if / else if / else .

Le lingue che potrebbero essere più adatte a questo tipo di struttura potrebbero includere SNOBOL4 (con la sua bizzarra ramificazione in stile GOTO) o linguaggi logici come Prolog e Mercury (con le loro capacità di unificazione e backtracking, non per menzionare i loro DCG per un'espressione piuttosto succinta di decisioni complicate).

Ovviamente se questa non è un'opzione (perché molti programmatori sono, tragicamente, non poliglotti) le idee che gli altri hanno escogitato sono buone come usare vari OOP strutture o suddividere le clausole in funzioni o anche, se sei disperato, e non preoccuparti del fatto che molte persone le trovino illeggibili, usando un macchina di stato .

La mia soluzione, tuttavia, continua a raggiungere un linguaggio che ti permetta di esprimere la logica che stai cercando di esprimere (supponendo che questo sia comune nel tuo codice) in un modo più semplice e più leggibile.

Dividerli in funzioni potrebbe essere d'aiuto?

Chiama la prima funzione e quando termina chiamerà la prossima, la prima cosa che la funzione farebbe è testare la verifica di quella funzione.

Dipende molto dalle dimensioni di ciascun blocco di codice e dalla dimensione totale, ma io tendo a suddividerlo sia con un "metodo di estrazione" diretto su un blocco, sia muovendo le parti incondizionate in metodi separati. Ma i caveat @KeesDijk citati si applicano. Il primo approccio ti offre una serie di funzioni come

if (Check#1)
{
    CodeBlock#1
    NextFunction
}

Che può funzionare bene ma porta a un aumento di codice e al "metodo usato una volta sola". Quindi generalmente preferisco questo approccio:

if (CheckFunctionOne)
{
    MethodOneWithDescriptiveName
    if (CheckFunctionTwo)
    {
        MethodTwoWithDescriptiveName
        ....

Con l'uso appropriato di variabili private e il passaggio dei parametri, è possibile farlo. Avere dato un'occhiata alla programmazione letterale può aiutarti qui.

Da qui :

Quite often when I'm looking at a set of pac-man ifs I find that if I just draw out something like a truth table of all the conditions involved I can work out a much better route to resolving the problem.

That way you can also assess whether there is a better method, how you might break it down further and ( and this is a big one with this kind of code ) whether there are any holes in the logic.

Having done that you can probably break it down into a couple of switch statements and a couple of methods and save the next poor mook who has to go through the code a whole lot of problems.

Personalmente, mi piace avvolgere queste istruzioni if in funzioni separate che restituiscono un valore bool se la funzione ha successo.

La struttura appare come segue:

if (DoCheck1AndCodeBlock1() && 
    DoCheck2AndCodeBlock2() && 
    DoCheck3AndCodeBlock3()) 
{
   // ... you may perform the final operation here ....
}

L'unico inconveniente è che queste funzioni dovranno generare dati usando attributi passati per riferimento o variabili membro.

Se vuoi una possibile soluzione orientata agli oggetti, il codice sembra un esempio canonico per il refactoring utilizzando la catena di responsabilità motivo di progettazione .