Qual è lo stile di codifica migliore?

Question

Durante una revisione del codice, uno sviluppatore senior ha commentato alcuni annidamenti che avevo avuto nel mio codice. Mi ha suggerito di impostare un valore bool in modo che non abbia mai più di un livello di annidamento. Penso che il mio codice sia più leggibile, ma voglio ottenere l'opinione degli altri sviluppatori su questo. Qual è lo stile migliore? La sua avversione istintiva alla nidificazione è fondata?

Di seguito sono riportati alcuni esempi di codice semplificato.

nidificati:

If(condition1)
{
    If(condition2)
    {
        if(condition3)
        {
            return true;
        }
        else
        {
            log("condition3 failed");
        }
    else
    {
        log("condition2 failed")
    }
}
else
{
    log("condition1 failed")
}

return false;

o

Bool Driven:

bool bRC = false;

bRC = (condition1);
if(brc)
{
    bRC = (condition2);
}
else
{
    log("condition1 failed");
    return false;
}

if(bRC)
{
    bRC = (condition3);
}
else
{
    log("condition2 failed");
    return false;
}

if(bRC)
{
    return true;
}
else
{
    log("condition3 failed");
    return false;
}

Answer 1

Mi piace di più il tuo, ma probabilmente farei qualcosa del genere:

if (condition1 && condition2 && condition3)
{
    return true;
}
else if (!condition1)
{
    log("condition1 failed");
}
else if (!condition2)
{
    log("condition2 failed");
}
else
{
    log("condition3 failed");
}
return false;

Se le condizioni sono espressioni complesse, allora potrei assegnare le espressioni a variabili con nome appropriato prima di valutare le istruzioni if ??per evitare di dover ricalcolare i valori in ciascuna if.

Questo presuppone che la modalità normale sia che tutte le condizioni sono vere e quindi si desidera prima avere quel controllo. Se la modalità normale è che una o più condizioni sono false, allora riordinerei e controllerei ogni negazione a sua volta e restituirei semplicemente vero se tutti i controlli fallissero. Ciò eliminerebbe anche la necessità che variabili temporanee prendano il posto di espressioni complesse.

Answer 2

Se non hai regole stupide su più punti di ritorno, penso che questo sia abbastanza carino (e così anche qualcun altro, ma hanno eliminato la loro risposta per motivi sconosciuti):

if(!condition1)
{
    log("condition1 failed");
    return false;
}

if(!condition2)
{
    log("condition2 failed");
    return false;
}

if(!condition3)
{
    log("condition3 failed");
    return false;
}

return true;

Forse questa è un'eguale avversione istintiva nei confronti del super-annidamento, ma è certamente più pulita della sua merda che memorizza le condizioni booleane in determinati valori. Tuttavia, potrebbe essere meno leggibile nel contesto: considera se una delle condizioni era isreadable () . È più chiaro dire if (isreadable ()) perché vogliamo sapere se qualcosa è leggibile. if (! isreadable ()) suggerisce se vogliamo sapere se non è leggibile, il che non è nostra intenzione. È certamente discutibile che possano esserci situazioni in cui una è più leggibile / intuitiva dell'altra, ma io stesso sono un fan di questo. Se qualcuno viene bloccato dai resi, puoi farlo:

if(!condition1)
    log("condition1 failed");

else if(!condition2)
    log("condition2 failed");

else if(!condition3)
    log("condition3 failed");

else
    return true;

return false;

Ma questo è piuttosto subdolo, e meno "chiaro". secondo me.

Answer 3

Personalmente trovo il codice nidificato molto più facile da leggere.

Answer 4

In genere preferisco nidificato per le mie condizioni; ovviamente, se le mie condizioni nidificate vengono rientrate troppo a destra, devo iniziare a chiedermi se c'è un modo migliore per fare ciò che sto cercando di fare (refactoring, riprogettazione, ecc.)

Answer 5

Simile alla versione nidificata, ma molto più pulito per me:

if not condition1:
    log("condition 1 failed")
else if not condition2:
    log("condition 2 failed")
else if not condition3:
    log("condition 3 failed")
else
    return true;
return false;

Attenzione che ogni condizione viene valutata una volta.

Answer 6

Il secondo stile è assurdamente dettagliato: ha davvero suggerito esattamente questo? Non hai bisogno della maggior parte di quelle if , perché c'è un return nella parte else.

Con l'esempio nidificato fai affidamento sul non dimenticare di includere eventuali clausole else .

Nessuno dei due mi sembra soddisfacente.

Answer 7

Il modo bool driven è confuso. L'annidamento va bene, se necessario, ma è possibile rimuovere parte della profondità dell'annidamento combinando le condizioni in un'istruzione o chiamando un metodo in cui viene eseguita una parte dell'ulteriore valutazione.

Answer 8

Penso che entrambi i modi siano possibili e abbiano i loro pro e contro. Userei lo stile bool-driven nei casi in cui avrei un annidamento molto profondo (8 o 10 o qualcosa del genere). Nel tuo caso con 3 livelli, sceglierei il tuo stile, ma per l'esatto campione dall'alto, andrei così:

void YourMethod(...)
{
  if (condition1 && condition2 && consition3)
    return true;

  if (!condition 1)
    log("condition 1 failed");

  if (!condition 2)
    log("condition 2 failed");

  if (!condition 3)
    log("condition 3 failed");

  return result;
}

... o così se preferisci un singolo punto di uscita (come faccio io) ...

void YourMethod(...)
{
  bool result = false;

  if (condition1 && condition2 && consition3)
  { 
    result = true;
  }
  else
  {
    if (!condition 1)
      log("condition 1 failed");

    if (!condition 2)
      log("condition 2 failed");

    if (!condition 3)
      log("condition 3 failed");
  }
  return result;
}

In questo modo, otterrai tutte le condizioni non riuscite registrate nella prima esecuzione. Nel tuo esempio, otterrai solo una condizione non riuscita registrata anche se ci sono più di una condizione non riuscita.

Answer 9

Probabilmente ci andrei con

   if (!condition1)      log("condition 1 failed");
   else if (!condition2) log("condition 2 failed");
   else if (!condition3) log("condition 3 failed");
   // -------------------------------------------
   return condition1 && condition2 && condition3;

che credo sia equivalente e molto più pulito ...

Inoltre, una volta che il client decide che tutte le condizioni dovrebbero essere valutate e registrate se falliscono, non solo la prima che fallisce, è molto più facile modificarlo per farlo:

   if (!condition1) log("condition 1 failed");
   if (!condition2) log("condition 2 failed");
   if (!condition3) log("condition 3 failed");
   // -------------------------------------------
   return condition1 && condition2 && condition3;

Answer 10

Ecco come lo implementerei, a condizione che le tue implementazioni riflettano effettivamente il comportamento desiderato.

if (!condition1) {
    log("condition1 failed");
    return false;
}
if (!condition2) {
    log("condition2 failed");
    return false;
}
if (!condition3) {
    log("condition3 failed");
    return false;
}
return true;

Tuttavia, ritengo sia più probabile che ogni condizione non riuscita venga registrata.

result = true;
if (!condition1) {
    log("condition1 failed");
    result = false;
}
if (!condition2) {
    log("condition2 failed");
    result = false;
}
if (!condition3) {
    log("condition3 failed");
    result = false;
}
return result;

Answer 11

Se la lingua supporta la gestione delle eccezioni, sceglierei quanto segue:

try {
    if (!condition1) {
        throw "condition1 failed";
    }

    if (!condition2) {
        throw "condition2 failed";
    }

    if (!condition3) {
        throw "condition3 failed";
    }

    return true;

} catch (e) {
    log(e);
    return false;
}

MODIFICA da charles bretana: vedi Uso delle eccezioni per il flusso di controllo

Answer 12

Non mi piace in entrambi i casi. Quando hai tanti nidi qualcosa non va. Nel caso di una convalida del modulo o qualcosa che effettivamente richiede qualcosa di simile, prova a capire qualcosa di più modulare o compatto.

Un esempio potrebbe essere un array che contiene le condizioni, attraverso le quali eseguirai l'iterazione per un po 'e stampa / interrompi secondo necessità.

Ci sono troppe implementazioni a seconda delle tue necessità, quindi creare un codice di esempio sarebbe inutile.

Come regola generale, quando il tuo codice sembra troppo complicato, fa schifo :). Prova a ripensarlo. Seguire le pratiche di codifica la maggior parte delle volte rende il codice molto più estetico e breve; e ovviamente anche più intelligente.

Answer 13

Il codice deve riaffermare il problema in una lingua data. Pertanto, sostengo che entrambi i frammenti possono essere "migliori". Dipende dal problema che viene modellato. Anche se la mia ipotesi è che nessuna delle soluzioni corrisponderà al problema reale. Se inserisci termini reali invece di condizione1,3, potrebbe cambiare completamente il "migliore" codice.
Sospetto che esista un modo migliore (3d) per scriverlo tutti insieme.

Answer 14

if( condition1 && condition2 && condition3 )
    return true;

log(String.Format("{0} failed", !condition1 ? "condition1" : (!condition2 ? "condition2" : "condition3")));
return false;

In questo modo, non è necessario visualizzare molte righe di codice solo per la registrazione. E se tutte le tue condizioni sono vere, non perdi tempo a valutarle nel caso in cui devi effettuare l'accesso.