Come affrontare un anti-pattern a freccia "ramificato"? [duplicare]

Il codice può essere semplificato in questo modo:

booleancondA=(fooSuccess&&!barSuccess&&mooSuccess)booleancondB=(fooSuccess&&!barSuccess&&!mooSuccess)booleancondC=(!fooSuccess&&barSuccess&&mooSuccess)booleancondD=(!fooSuccess&&barSuccess&&!mooSuccess&&cowSuccess)booleancondE=(!fooSuccess&&barSuccess&&!mooSuccess&&!cowSuccess)if(condA){....return;}if(condB){....return;}...andsosonif(condE){....return;}

Insostanza:

• Valutacondizionicomplesseinunsingolobooleano
• Dalmomentochelecondizionisonoesclusive(comeaccadetipicamentenelletestedifrecceannidate)nondevinidificareiif,bastaaggiungereunritornoinmodochenonvengaeseguitonessunaltroblocco.
• Ciòriduceenormementelacomplessitàciclomaticaerendebanaleilcodice.
• Avendoesattamenteciòchevieneeseguitoinognicaso,puoianchefareuna tabella Karnaugh per identificare e rimuovere le combinazioni di condizioni ridondanti , proprio come fanno nel design dei circuiti logici. Ma non l'hai fornito nell'esempio.
• Assicurati di estrarre tutta questa logica nel proprio metodo in modo che le istruzioni di ritorno non interferiscano con i blocchi che devono essere eseguiti in qualsiasi momento.
• I nomi descrittivi dovrebbero essere scelti per conA..condF .

user61852 ha una buona risposta che risolve il problema generale di semplificare i condizionali nidificati. Tuttavia, sono rimasto affascinato dalla tua soluzione di macchina simile a quella dello stato e volevo illustrare come a volte può essere preferibile a un set di flag binari.

Tutto dipende dalla sequenza di ottenere i valori dei flag e da quante combinazioni sono valide. Se hai n flags, hai 2 stati ⁿ . Per 4 flag, si tratta di 16 stati e, come hai osservato, solo 3 di essi potrebbero avere un significato utile. In situazioni del genere, l'uso di una variabile di stato invece può semplificare notevolmente le cose.

Diciamo che hai un lucchetto che si aprirà se vengono premuti 4 tasti nell'ordine corretto. Se si preme un tasto in modo errato, si reimposta immediatamente all'inizio della sequenza. Un modo molto ingenuo per implementare un gestore di keypress usando i flag binari è:

void key_pressed(char key)
{
   if (!key1correct)
   {
      if (key == pin[0])
      {
         key1correct = true;
      }
   }
   else if (!key2correct)
   {
       if (key == pin[1])
       {
           key2correct = true;
       }
       else
       {
           key1correct = false;
       }
   }
   else if (!key3correct)
   {
       if (key == pin[2])
       {
           key3correct = true;
       }
       else
       {
           key1correct = false;
           key2correct = false;
       }
   }
   else
   {
       if (key == pin[3])
       {
           key4correct = true;
       }
       else
       {
           key1correct = false;
           key2correct = false;
           key3correct = false;
       }
   }

   if (key1correct && key2correct && key3correct && key4correct)
   {
       open_lock();
       key1correct = false;
       key2correct = false;
       key3correct = false;
       key4correct = false;
   }
}

Una versione semplificata e appiattita che usa i flag binari (come la risposta dell'utente61852) assomiglia a questo:

void key_pressed(char key)
{
    if (!key1correct && key == pin[0])
    {
        key1correct = true;
        return;
    }

    if (key1correct && !key2correct && key == pin[1])
    {
        key2correct = true;
        return;
    }

    if (key1correct && key2correct && !key3correct && key == pin[2])
    {
        key3correct = true;
        return;
    }

    if (key1correct && key2correct && key3correct && key == pin[3])
    {
        open_lock();
        key1correct = false;
        key2correct = false;
        key3correct = false;
        return;
    }

    key1correct = false;
    key2correct = false;
    key3correct = false;
}

È molto più facile da leggere, ma in entrambe queste soluzioni hai stati come key2correct && !key1correct completamente non validi e non utilizzati. Mentre l'uso di una variabile di stato num_correct_keys invece dei flag binari ha il seguente aspetto:

void key_pressed(char key)
{
    if (key == pin[num_correct_keys])
        ++num_correct_keys;
    else
        num_correct_keys = 0;

    if (num_correct_keys == 4)
    {
        open_lock();
        num_correct_keys = 0;
    }
}

Certo, questo è un esempio forzato, ma le persone spesso scrivono codice come la prima versione in situazioni meno ovvie, a volte distribuite su più file. L'uso di una variabile di stato spesso semplifica enormemente il codice, specialmente quando i flag rappresentano una sequenza di eventi.

Non sono sicuro che il tuo esempio sia particolarmente adatto a questa domanda, ma è possibile condensarlo in modo molto più semplice e mantenere la stessa logica:

if(fooSuccess && !barSuccess)
{
    if(mooSuccess)
    {
        // .....
    }
    else
    {
        // .....
    }
}
else if (!fooSuccess && barSuccess)
{
    if(mooSuccess)
    {
        // .....
    }
    else if(cowSuccess)
    {
        // .....
    }
    else 
    {
        // .....
    }
}

Nota come non c'è più di un livello in profondità di if s. Per pulire questo, ho preso i seguenti passi:

• Non verificare mai se un valore booleano è == true , != true o != false . Occasionalmente ho scoperto che vale la pena verificare se è == false per rendere più evidente che è il caso previsto, ma anche quello dovrebbe essere eccezionale.
• Se il tuo blocco else non consiste di nient'altro che un blocco if / else , puoi quindi unirlo con il genitore else .
• Se la tua istruzione if ha solo un input / variabile, non devi controllare l'inverso per else .