Ist es eine schlechte Praxis, return innerhalb einer void-Methode zu verwenden?

87

Stellen Sie sich den folgenden Code vor:

void DoThis()
{
    if (!isValid) return;

    DoThat();
}

void DoThat() {
    Console.WriteLine("DoThat()");
}

Ist es in Ordnung, eine Rückgabe innerhalb einer void-Methode zu verwenden? Hat es eine Leistungsstrafe? Oder es wäre besser, einen Code wie diesen zu schreiben:

void DoThis()
{
    if (isValid)
    {
        DoThat();
    }
}

quelle
1
Was ist mit: void DoThis () {if (isValid) DoThat (); }
Dscoduc
28
Stell dir den Code vor? Warum? Es ist genau da! :-D
STW
Dies ist eine gute Frage, ich denke immer, es ist eine gute Praxis, return zu verwenden. um die Methode oder Funktion zu verlassen. Insbesondere bei einer LINQ-Data-Mining-Methode mit mehreren IQueryable <T> -Ergebnissen hängen alle voneinander ab. Wenn einer von ihnen kein Ergebnis hat, alarmieren und beenden.
Cheung

Antworten:

32

Es gibt noch einen weiteren wichtigen Grund für die Verwendung von Guards (im Gegensatz zu verschachteltem Code): Wenn ein anderer Programmierer Ihrer Funktion Code hinzufügt, arbeiten diese in einer sichereren Umgebung.

Erwägen:

void MyFunc(object obj)
{
    if (obj != null)
    {
        obj.DoSomething();
    }
}

gegen:

void MyFunc(object obj)
{
    if (obj == null)
        return;

    obj.DoSomething();
}

Stellen Sie sich nun vor, ein anderer Programmierer fügt die Zeile hinzu: obj.DoSomethingElse ();

void MyFunc(object obj)
{
    if (obj != null)
    {
        obj.DoSomething();
    }

    obj.DoSomethingElse();
}

void MyFunc(object obj)
{
    if (obj == null)
        return;

    obj.DoSomething();
    obj.DoSomethingElse();
}

Dies ist natürlich ein vereinfachter Fall, aber der Programmierer hat dem Programm in der ersten Instanz (verschachtelter Code) einen Absturz hinzugefügt. Im zweiten Beispiel (vorzeitiger Ausstieg mit Wachen) ist Ihr Code vor unbeabsichtigter Verwendung einer Nullreferenz geschützt, sobald Sie an der Wache vorbeikommen.

Sicher, ein großartiger Programmierer macht solche Fehler (oft) nicht. Aber Vorbeugen ist besser als Heilen - wir können den Code so schreiben, dass diese potenzielle Fehlerquelle vollständig beseitigt wird. Das Verschachteln erhöht die Komplexität. Daher empfehlen Best Practices, Code umzugestalten, um das Verschachteln zu reduzieren.

Jason Williams
quelle
Ja, aber auf der anderen Seite machen mehrere Verschachtelungsebenen mit ihren Bedingungen den Code noch anfälliger für Fehler, die Logik schwieriger zu verfolgen und - was noch wichtiger ist - schwieriger zu debuggen. Flache Funktionen sind weniger böse, IMO.
Skrim
17
Ich streite in zugunsten der reduzierten Verschachtelung! :-)
Jason Williams
Ich stimme dem zu. Unter dem Gesichtspunkt des Refactors ist es außerdem einfacher und sicherer, die Methode zu refactorisieren, wenn obj zu einer Struktur wird oder etwas, von dem Sie garantieren können, dass es nicht null ist.
Phil Cooper
18

Schlechte Praxis ??? Auf keinen Fall. Tatsächlich ist es immer besser, Validierungen zu behandeln, indem Sie frühestens von der Methode zurückkehren, wenn die Validierungen fehlschlagen. Andernfalls würde es zu einer großen Anzahl verschachtelter Wenns und Sonstiges kommen. Ein vorzeitiges Beenden verbessert die Lesbarkeit des Codes.

Überprüfen Sie auch die Antworten auf eine ähnliche Frage: Soll ich die return / continue-Anweisung anstelle von if-else verwenden?

SO Benutzer
quelle
7

Es ist keine schlechte Praxis (aus allen bereits genannten Gründen). Je mehr Renditen eine Methode hat, desto wahrscheinlicher sollte sie in kleinere logische Methoden aufgeteilt werden.

Mike Hall
quelle
6

Das erste Beispiel verwendet eine Guard-Anweisung. Aus Wikipedia :

Bei der Computerprogrammierung ist ein Guard ein boolescher Ausdruck, der als wahr ausgewertet werden muss, wenn die Programmausführung in dem betreffenden Zweig fortgesetzt werden soll.

Ich denke, ein paar Wachen an der Spitze einer Methode zu haben, ist eine vollkommen verständliche Art zu programmieren. Grundsätzlich heißt es: "Führen Sie diese Methode nicht aus, wenn eine dieser Methoden zutrifft."

Im Allgemeinen würde es also so aussehen:

void DoThis()
{
  if (guard1) return;
  if (guard2) return;
  ...
  if (guardN) return;

  DoThat();
}

Ich denke, das ist dann viel besser lesbar:

void DoThis()
{
  if (guard1 && guard2 && guard3)
  {
    DoThat();
  }
}
cdmckay
quelle
3

Es gibt keine Leistungseinbußen, der zweite Code ist jedoch besser lesbar und daher einfacher zu warten.

Russell
quelle
Russell, ich bin mit Ihrer Meinung nicht einverstanden, aber Sie hätten nicht dafür herabgestimmt werden sollen. +1, um es auszugleichen. Übrigens glaube ich, dass ein boolescher Test und die Rückkehr in einer einzelnen Zeile, gefolgt von einer leeren Zeile, ein klarer Hinweis darauf ist, was los ist. zB Rodrigos erstes Beispiel.
Paul Sasik
Ich bin damit nicht einverstanden. Das Erhöhen der Verschachtelung verbessert die Lesbarkeit nicht. Der erste Code verwendet eine "Guard" -Anweisung, die ein vollkommen verständliches Muster darstellt.
CDMckay
Ich bin auch anderer Meinung. Schutzklauseln, die frühzeitig aus einer Funktion aussteigen, werden heutzutage allgemein als eine gute Sache angesehen, um dem Leser zu helfen, die Implementierung zu verstehen.
Pete Hodgson
2

In diesem Fall ist Ihr zweites Beispiel besserer Code, aber das hat nichts mit der Rückkehr von einer void-Funktion zu tun, sondern einfach, weil der zweite Code direkter ist. Aber die Rückkehr von einer leeren Funktion ist völlig in Ordnung.

Imagist
quelle
1

Es ist vollkommen in Ordnung und keine Leistungseinbußen, aber schreiben Sie niemals eine Wenn-Anweisung ohne Klammern.

Immer

if( foo ){
    return;
}

Es ist viel besser lesbar; und Sie werden niemals versehentlich davon ausgehen, dass einige Teile des Codes in dieser Anweisung enthalten sind, wenn dies nicht der Fall ist.

Mittags Seide
quelle
2
lesbar ist subjektiv. Imho, alles, was zu unnötigem Code hinzugefügt wird, macht ihn weniger lesbar ... (Ich muss mehr lesen, und dann frage ich mich, warum er da ist und verschwende Zeit damit, sicherzustellen, dass mir nichts fehlt) ... aber das ist mein subjektive Meinung
Charles Bretana
10
Der bessere Grund, die Zahnspangen immer einzuschließen, ist weniger die Lesbarkeit als vielmehr die Sicherheit. Ohne die Klammern ist es für jemanden später zu einfach, einen Fehler zu beheben, der zusätzliche Anweisungen als Teil des if erfordert. Achten Sie nicht genau genug darauf und fügen Sie sie hinzu, ohne auch die Klammern hinzuzufügen. Durch das ständige Einbeziehen der Zahnspangen wird dieses Risiko beseitigt.
Scott Dorman
2
Seidig, bitte drücken Sie die Eingabetaste vor Ihrem {. Dies stimmt {mit Ihrer }in derselben Spalte überein, was die Lesbarkeit erheblich verbessert (viel einfacher, entsprechende Klammern zum Öffnen / Schließen zu finden).
Imagist
@Imagist Ich überlasse das den persönlichen Vorlieben; und es ist so gemacht, wie ich es bevorzuge :)
Noon Silk
1
Wenn jede geschlossene Klammer mit einer offenen Klammer übereinstimmt, die auf derselben Einrückungsstufe platziert ist , ist ifes einfach, visuell zu unterscheiden, welche Anweisungen enge Klammern erfordern, und daher ifist es sicher, wenn Anweisungen eine einzelne Anweisung steuern. Durch Zurückschieben der offenen Klammer auf die Linie mit ifspart eine Linie vertikalen Raums bei jeder Mehrfachanweisung if, erfordert jedoch die Verwendung einer ansonsten unnötigen engen Klammer.
Supercat
0

Ich werde mit all Ihren jungen Whippersnappern in dieser Sache nicht einverstanden sein.

Die Verwendung von Return mitten in einer Methode, ungültig oder auf andere Weise, ist aus Gründen, die der verstorbene Edsger W. Dijkstra vor fast vierzig Jahren ganz klar formuliert hat, sehr schlecht, beginnend mit der bekannten "GOTO Statement Considered Harmful" "und weiter in" Structured Programming "von Dahl, Dijkstra und Hoare.

Die Grundregel lautet, dass jede Kontrollstruktur und jedes Modul genau einen Eintrag und einen Ausgang haben sollte. Eine explizite Rückgabe in der Mitte des Moduls verstößt gegen diese Regel und macht es viel schwieriger, über den Status des Programms nachzudenken, was es wiederum viel schwieriger macht zu sagen, ob das Programm korrekt ist oder nicht (was eine viel stärkere Eigenschaft ist als "ob es zu funktionieren scheint oder nicht").

"GOTO Statement als schädlich" und "Structured Programming" leiteten die Revolution der "Structured Programming" der 1970er Jahre ein. Diese beiden Teile sind die Gründe, warum wir heute if-then-else, while-do und andere explizite Kontrollkonstrukte haben und warum GOTO-Anweisungen in Hochsprachen auf der Liste der gefährdeten Arten stehen. (Meine persönliche Meinung ist, dass sie auf der Liste der ausgestorbenen Arten stehen müssen.)

Es ist erwähnenswert, dass der Message Flow Modulator, die erste militärische Software, die EVER beim ersten Versuch ohne Abweichungen, Verzichtserklärungen oder "Ja, aber" -Sprache bestanden hat, in einer Sprache geschrieben wurde, die es nicht einmal gab eine GOTO-Anweisung.

Erwähnenswert ist auch, dass Nicklaus Wirth die Semantik der RETURN-Anweisung in Oberon-07, der neuesten Version der Oberon-Programmiersprache, geändert hat und sie zu einem nachfolgenden Teil der Deklaration einer typisierten Prozedur (dh einer Funktion) und nicht zu einer ausführbare Anweisung im Hauptteil der Funktion. Seine Erklärung der Änderung sagte , dass er gerade tat , weil die bisherige Form WAR eine Verletzung des Ein-Ausgang Prinzip der strukturierten Programmierung.

John R. Strohm
quelle
2
@ John: Wir haben die Dykstra-einstweilige Verfügung wegen mehrfacher Rückgaben ungefähr zu dem Zeitpunkt überwunden, als wir über Pascal hinweg waren (die meisten von uns jedenfalls).
John Saunders
Fälle, in denen mehrere Rückgaben erforderlich sind, sind häufig Anzeichen dafür, dass eine Methode versucht, zu viel zu tun, und sollten reduziert werden. Ich werde damit nicht so weit wie John gehen, und eine return-Anweisung als Teil der Parametervalidierung mag eine vernünftige Ausnahme sein, aber ich verstehe, woher die Idee kommt.
Kyoryu
@ Nairdaen: Es gibt immer noch Kontroversen über Ausnahmen in diesem Quartal. Meine Richtlinie lautet: Wenn das in der Entwicklung befindliche System das Problem beheben MUSS, das den ursprünglichen Ausnahmezustand verursacht hat, und es mir nichts ausmacht, den Kerl zu verärgern, der diesen Code schreiben muss, werde ich eine Ausnahme auslösen. Dann werde ich in einer Besprechung angeschrien, weil der Typ sich nicht die Mühe gemacht hat, die Ausnahme zu erwischen, und die App beim Testen abgestürzt ist, und ich erkläre, WARUM er das Problem beheben muss und sich die Dinge wieder beruhigen.
John R. Strohm
Es gibt einen großen Unterschied zwischen Wachaussagen und Gotos. Das Böse von Gotos ist, dass sie überall springen können, daher kann es sehr verwirrend sein, sich zu entwirren und sich zu erinnern. Guard-Anweisungen sind genau das Gegenteil - sie bieten einen gated Eintrag zu einer Methode, nach der Sie wissen, dass Sie in einer "sicheren" Umgebung arbeiten, wodurch die Anzahl der Dinge reduziert wird, die Sie beim Schreiben des restlichen Codes berücksichtigen müssen (z "Ich weiß, dass dieser Zeiger niemals null sein wird, daher muss ich diesen Fall nicht im gesamten Code behandeln").
Jason Williams
@ Jason: Die ursprüngliche Frage betraf nicht speziell Guard-Anweisungen, sondern zufällige Rückgabeanweisungen mitten in einer Methode. Das gegebene Beispiel schien eine Wache zu sein. Das Hauptproblem ist, dass Sie an der Rückgabestelle in der Lage sein möchten, darüber nachzudenken, was die Methode getan oder nicht getan hat, und zufällige Rückgaben machen dies schwieriger, genau aus den gleichen Gründen, aus denen zufällige GOTOs es schwieriger machen. Siehe: Dijkstra, "GOTO-Erklärung als schädlich angesehen". Auf der Syntaxseite gab cdmckay in einer anderen Antwort seine bevorzugte Syntax für Wachen an; Ich bin nicht einverstanden mit seiner Meinung, welche Form besser lesbar ist.
John R. Strohm
-3

Ausnahme auslösen, anstatt nichts zurückzugeben, wenn das Objekt null ist usw.

Ihre Methode erwartet, dass das Objekt nicht null ist und dies nicht der Fall ist. Sie sollten daher eine Ausnahme auslösen und den Aufrufer damit umgehen lassen.

Aber eine frühzeitige Rückkehr ist sonst keine schlechte Praxis.

Dhananjay
quelle
Die Antwort beantwortet die Frage nicht. Die Frage ist eine ungültige Methode, daher wird nichts zurückgegeben. Außerdem haben die Methoden keine Parameter. Ich bekomme den Punkt, nicht null zurückzugeben, wenn der Rückgabetyp ein Objekt ist, aber das gilt nicht für diese Frage.
Luke Hammer