Welches ist besser Codierungsstil?

Question

Während ein Code-Review, ein leitender Entwickler kommentierte einige Verschachtelung ich in meinem Code los war. Er schlug vor, ich einen Bool Wert gesetzt, so dass ich nie mehr als eine Ebene der Verschachtelung haben. Ich glaube, mein Code besser lesbar ist, aber will andere Devs' Meinung zu diesem Thema erhalten. Welches ist besser Stil? Ist seine reflexAversion begründet Verschachtelung?

Im Folgenden sind einige vereinfachte Code-Beispiele.

Verschachtelte:

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

return false;

oder

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

Ich mag Sie besser, aber ich würde wahrscheinlich so etwas wie:

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

Wenn die Bedingungen sind komplexe Ausdrücke dann könnte ich die Ausdrücke zuweisen geeignete Variablen mit dem Namen, bevor der Auswertung, wenn Aussagen zu vermeiden, um die Werte in jedem neu zu berechnen, wenn.

Dies wird unter der Annahme, dass der normale Modus ist, dass alle Bedingungen erfüllt sind und so wollen Sie zuerst, dass der Check haben. Wenn der normale Modus ist, dass eine oder mehr Bedingungen falsch sind, dann würde ich es neu anordnen und jede Negation wiederum prüfen und einfach wahr zurück, wenn alle Kontrollen versagt. Das würde entfernen auch die Notwendigkeit für temporäre Variablen an die Stelle komplexer Ausdrücke zu nehmen.

Answer 2

Wenn Sie keine dummen Regeln über mehrere Rückkehrpunkte haben, ich denke, das ist ganz nett (und so auch jemand anderes, aber sie gelöscht ihre Antwort aus unbekannten Gründen):

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

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

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

return true;

Vielleicht ist dies eine gleich reflex Abneigung gegen Super-Verschachtelung, aber es ist auf jeden Fall sauberer als sein Mist die Booleschen Bedingungen in bestimmten Werten zu speichern. Allerdings kann es sein, weniger lesbar in Zusammenhang: prüfen, ob eine der Bedingungen isreadable() wurde. Es ist klarer if(isreadable()) zu sagen, weil wir wissen wollen, ob etwas lesbar ist. if(!isreadable()) legt nahe, wenn wir wissen wollen, ob es nicht lesbar ist, was nicht unsere Absicht ist. Es ist sicherlich fraglich, dass es Situationen geben kann, in denen ein lesbaren / intuitiv als die andere ist, aber ich bin ein Fan dieser Art und Weise selbst. Wenn jemand wird auf die Rendite auflegte, Sie könnten dies tun:

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

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

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

else
    return true;

return false;

Aber das ist eher hinterhältig und weniger „klar“ meiner Meinung nach.

Answer 3

ich persönlich den verschachtelten Code wesentlich einfacher finden zu lesen.

Answer 4

ich in der Regel lieber für meine Bedingungen verschachtelt; natürlich, wenn meine verschachtelten Bedingungen sind zu weit nach rechts eingerückt zu werden, muß ich fragen beginnen, ob es ein besserer Weg zu gehen, was auch immer ich versuche (Refactoring, Redesign, etc ..) zu tun

Answer 5

ähnlich wie die verschachtelten Version, aber viel sauberer für mich:

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;

Beachten Sie, dass jede Bedingung einmal ausgewertet wird.

Answer 6

Die zweite Art ist absurd ausführliche: hat er vorschlagen, genau das wirklich? Sie brauchen nicht die meisten dieser if Aussagen, weil es eine return im else-Teil ist.

Mit dem verschachtelten Beispiel Sie verlassen sich zu vergessen, auf keine mögliche else Klauseln enthalten.

Beides scheint zufriedenstellend zu mir.

Answer 7

Der Bool gesteuert ist verwirrend. Verschachtelung ist in Ordnung, wenn erforderlich, aber man konnte einige der Schachtelungstiefe entfernen, indem die Bedingungen in einer Anweisung kombinieren, oder eine Methode aufrufen, wo einige der weiteren Auswertung durchgeführt wird.

Answer 8

ich denke, beide Wege sind möglich und haben ihre Vor- und Nachteile. Ich würde den Bool-driven Stil in Fällen verwenden, in denen ich wirklich tief Verschachtelung haben würde (8 oder 10 oder so ähnlich). In Ihrem Fall mit 3 Ebenen, würde ich Ihren Stil wählen, aber für die exakte Probe von oben, ich würde so weitergehen:

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;
}

... oder möchten, dass, wenn Sie es vorziehen, einen einzigen Austrittspunkt (wie ich) ...

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;
}

Auf diese Weise erhalten Sie alle Fehlerbedingung im ersten Lauf angemeldet bekommen. In Ihrem Beispiel, erhalten Sie nur eine Fehlerbedingung protokolliert, auch wenn es mehr als ein Versagen Bedingungen.

Answer 9

Ich würde wahrscheinlich gehen mit

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

, die ich glaube, ist equivilent und viel sauberer ...

Auch, wenn der Kunde entscheidet, dass alle Bedingungen ausgewertet und protokolliert werden sollen, wenn sie versagen, nicht nur die ersten, das, das ist viel einfacher zu ändern zu tun dies fehlschlägt:

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

Answer 10

Dies ist, wie ich es umsetzen würde, vorausgesetzt, Ihre Implementierungen tatsächlich das gewünschte Verhalten reflektieren.

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

Aber ich denke, es wahrscheinlicher ist, dass jede Fehlerbedingung protokolliert werden soll.

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

Wenn die Sprache Ausnahmebehandlung unterstützt, ich mit folgendem gehen würde:

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

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

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

    return true;

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

EDIT Von Charles Bretana: Bitte finden Sie unter Ausnahmen für die Ablaufsteuerung Mit

Answer 12

Ich mag es nicht so oder so. Wenn Sie so viele Nester haben, ist etwas nicht in Ordnung. Im Fall einer Formular-Validierung oder etwas, das in der Tat so etwas wie dies erfordert versuchen, etwas herauszufinden, dass mehr modular oder kompakt ist.

Ein Beispiel wäre ein Array sein, die Bedingungen zu halten, durch die man mit einer Weile laufen werden, und Druck / Pause je nach Bedarf.

Es gibt zu viele Implementierungen je nach Ihren Bedürfnissen so ein Beispiel-Code zu schaffen wäre sinnlos.

Als Faustregel gilt, wenn Ihr Code zu kompliziert aussieht, ist es saugt :). Versuchen Sie es zu überdenken. Nach Codierpraktiken die meisten der Zeit macht den Code viel mehr Ästhetik und kurz ausfallen; und natürlich auch intelligenter.

Answer 13

-Code wird das Problem in einer Sprache gegeben neu formulieren. Deshalb behaupte ich, dass entweder Schnipsel „besser“ sein kann. Es hängt davon ab, das Problem modelliert wird. Obwohl meine Vermutung ist, dass das eigentliche Problem weder die Lösungen werden parallel. Wenn Sie statt condition1,2,3 real setzen könnte es vollständig den „besten“ Code ändern.
Ich vermute, es ist eine bessere (3d) Art und Weise, dass alle zusammen zu schreiben.

Answer 14

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

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

Auf diese Weise haben Sie nicht viele Zeilen Code zu sehen, nur für die Anmeldung. Und wenn alle Bedingungen erfüllt sind, Sie verschwenden keine Zeit, sie bei der Bewertung nach Login müssen.