Was sind einige bessere Möglichkeiten, um das Do-While (0) zu vermeiden? in C ++ hacken?

233

Wenn der Code-Fluss so ist:

if(check())
{
  ...
  ...
  if(check())
  {
    ...
    ...
    if(check())
    {
      ...
      ...
    }
  }
}

Ich habe allgemein gesehen, wie dies funktioniert, um den oben genannten unordentlichen Codefluss zu vermeiden:

do {
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
} while(0);

Was sind einige bessere Möglichkeiten, um diese Umgehung / diesen Hack zu vermeiden, damit er zu einem Code auf höherer Ebene (Branchenebene) wird?

Vorschläge, die sofort einsatzbereit sind, sind willkommen!

Sankalp
quelle
38
RAII und Ausnahmen werfen.
ta.speot.is
135
Für mich klingt dies nach einem guten Zeitpunkt goto- aber ich bin sicher, jemand wird mich dafür markieren, dass ich das vorschlage, also schreibe ich keine Antwort auf diesen Effekt. LANG zu haben do ... while(0);scheint das Falsche zu sein.
Mats Petersson
42
@dasblinkenlight: Ja, in der Tat. Wenn Sie verwenden wollen goto, seien Sie ehrlich und tun Sie es im Freien, verstecken Sie es nicht mit breakunddo ... while
Mats Petersson
44
@MatsPetersson: Machen Sie eine Antwort, ich werde Ihnen +1 für Ihre gotoRehabilitationsbemühungen geben. :)
Wilx
27
@ ta.speot.is: "RAII und Ausnahmen auslösen". Auf diese Weise emulieren Sie die Flusskontrolle mit Ausnahmen. Das heißt, es ist ein bisschen so, als würde man teure, hochmoderne Hardware als Hammer oder Briefbeschwerer verwenden. Sie können das tun, aber das sieht definitiv nach einem sehr schlechten Geschmack für mich aus.
SigTerm

Antworten:

309

Es wird als akzeptable Praxis angesehen, diese Entscheidungen in einer Funktion zu isolieren und returnstattdessen s zu verwendenbreak s zu verwenden. Während alle diese Überprüfungen der gleichen Abstraktionsebene wie die Funktion entsprechen, ist dies ein logischer Ansatz.

Beispielsweise:

void foo(...)
{
   if (!condition)
   {
      return;
   }
   ...
   if (!other condition)
   {
      return;
   }
   ...
   if (!another condition)
   {
      return;
   }
   ... 
   if (!yet another condition)
   {
      return;
   }
   ...
   // Some unconditional stuff       
}
Mikhail
quelle
22
@MatsPetersson: "In Funktion isolieren" bedeutet das Umgestalten in eine neue Funktion, die nur die Tests ausführt.
MSalters
35
+1. Dies ist auch eine gute Antwort. In C ++ 11 kann die isolierte Funktion ein Lambda sein, da sie auch die lokalen Variablen erfassen kann, was die Sache einfacher macht!
Nawaz
4
@deworde: Je nach Situation wird diese Lösung möglicherweise viel länger und weniger lesbar als goto. Da C ++ leider keine lokalen Funktionsdefinitionen zulässt, müssen Sie diese Funktion (die, von der Sie zurückkehren werden) an einen anderen Ort verschieben, was die Lesbarkeit beeinträchtigt. Es kann dann zu Dutzenden von Parametern kommen. Da es eine schlechte Sache ist, Dutzende von Parametern zu haben, entscheiden Sie sich, diese in struct zu packen, wodurch ein neuer Datentyp erstellt wird. Zu viel Tippen für eine einfache Situation.
SigTerm
24
@Damon returnist sauberer, weil jeder Leser sofort merkt, dass es richtig funktioniert und was es tut. Mit gotoman muss schauen , um zu sehen , was es ist, und um sicher zu sein , dass keine Fehler gemacht wurden. Die Verkleidung ist der Vorteil.
R. Martinho Fernandes
11
@SigTerm: Manchmal sollte Code in eine separate Funktion verschoben werden, um die Größe jeder Funktion auf eine leicht zu begründende Größe zu beschränken. Wenn Sie für einen Funktionsaufruf nicht bezahlen möchten, markieren Sie ihn forceinline.
Ben Voigt
257

Es gibt Zeiten, in denen die Verwendung gototatsächlich die RICHTIGE Antwort ist - zumindest für diejenigen, die nicht in der religiösen Überzeugung erzogen sind, dass "goto niemals die Antwort sein kann, egal was die Frage ist" - und dies ist einer dieser Fälle.

Dieser Code verwendet den Hack von nur do { ... } while(0);zum Verkleiden eines gotoals break. Wenn Sie verwenden werdengoto , dann seien Sie offen darüber. Es macht keinen Sinn, den Code schwerer zu lesen.

Eine besondere Situation ist nur dann, wenn Sie viel Code mit recht komplexen Bedingungen haben:

void func()
{
   setup of lots of stuff
   ...
   if (condition)
   {
      ... 
      ...
      if (!other condition)
      {
          ...
          if (another condition)
          {
              ... 
              if (yet another condition)
              {
                  ...
                  if (...)
                     ... 
              }
          }
      }
  .... 

  }
  finish up. 
}

Es kann tatsächlich klarer machen, dass der Code korrekt ist, wenn keine so komplexe Logik vorhanden ist.

void func()
{
   setup of lots of stuff
   ...
   if (!condition)
   {
      goto finish;
   }
   ... 
   ...
   if (other condition)
   {
      goto finish;
   }
   ...
   if (!another condition)
   {
      goto finish;
   }
   ... 
   if (!yet another condition)
   {
      goto finish;
   }
   ... 
   .... 
   if (...)
         ...    // No need to use goto here. 
 finish:
   finish up. 
}

Bearbeiten: Zur Verdeutlichung schlage ich keineswegs die Verwendung gotoals allgemeine Lösung vor. Aber es gibt Fälle, in denengoto eine bessere Lösung als andere Lösungen vorliegt.

Stellen Sie sich zum Beispiel vor, wir sammeln einige Daten und die verschiedenen Bedingungen, auf die getestet wird, sind eine Art "Dies ist das Ende der gesammelten Daten" - was von einer Art von "Weiter / Ende" -Markierungen abhängt, die je nach Ort variieren Sie befinden sich im Datenstrom.

Wenn wir fertig sind, müssen wir die Daten in einer Datei speichern.

Und ja, es gibt oft andere Lösungen, die eine vernünftige Lösung bieten können, aber nicht immer.

Mats Petersson
quelle
76
Nicht zustimmen. gotomag einen Platz haben, goto cleanuptut es aber nicht. Die Bereinigung erfolgt mit RAII.
MSalters
14
@MSalters: Dies setzt voraus, dass die Bereinigung etwas beinhaltet, das mit RAII gelöst werden kann. Vielleicht hätte ich stattdessen "Problemfehler" oder ähnliches sagen sollen.
Mats Petersson
25
Also, immer wieder Downvotes zu diesem Thema, vermutlich von denen des religiösen Glaubens, gotoist nie die richtige Antwort. Ich würde mich freuen, wenn es einen Kommentar gäbe ...
Mats Petersson
19
Menschen hassen es, gotoweil man darüber nachdenken muss, es zu verwenden / ein Programm zu verstehen, das es verwendet ... Mikroprozessoren hingegen bauen darauf auf jumpsund conditional jumps... das Problem liegt also bei einigen Menschen, nicht bei Logik oder etwas anderem.
Woliveirajr
17
+1 für die angemessene Verwendung von goto. RAII ist nicht die richtige Lösung, wenn Fehler erwartet werden (keine Ausnahme), da dies ein Missbrauch von Ausnahmen wäre.
Joe
82

Sie können ein einfaches Fortsetzungsmuster mit einer boolVariablen verwenden:

bool goOn;
if ((goOn = check0())) {
    ...
}
if (goOn && (goOn = check1())) {
    ...
}
if (goOn && (goOn = check2())) {
    ...
}
if (goOn && (goOn = check3())) {
    ...
}

Diese Ausführungskette wird beendet, sobald checkNa zurückgegeben wird false. Keine weiteren check...()Anrufe würden durch Kurzschließen der durchgeführt werden&& Betreibers . Darüber hinaus sind optimierende Compiler intelligent genug, um zu erkennen, dass es sich bei der Einstellung goOnauf falseeine Einbahnstraße handelt, und die fehlenden goto endfür Sie einzufügen . Infolgedessen wäre die Leistung des obigen Codes mit der eines do/ identisch while(0), nur ohne dass die Lesbarkeit schmerzhaft beeinträchtigt würde.

dasblinkenlight
quelle
30
Zuweisungen innerhalb von ifBedingungen sehen sehr verdächtig aus.
Mikhail
90
@Mikhail Nur für ein ungeübtes Auge.
Dasblinkenlight
20
Ich habe diese Technik schon einmal verwendet und es hat mich immer irgendwie gestört, dass ich dachte, der Compiler müsste Code generieren, um jeden zu überprüfen, ifegal was goOnwar, selbst wenn ein früher fehlschlug (im Gegensatz zum Springen / Ausbrechen) ... aber ich habe gerade einen Test durchgeführt und VS2012 war zumindest klug genug, um sowieso alles nach dem ersten Falsch kurzzuschließen. Ich werde das öfter benutzen. Anmerkung: Wenn Sie verwenden goOn &= checkN()dann checkN()immer noch laufen, wenn goOnwar falsezu Beginn des if(dh das nicht tun).
Markieren Sie den
11
@Nawaz: Wenn Sie einen ungeübten Verstand haben, der willkürliche Änderungen an einer Codebasis vornimmt, haben Sie ein viel größeres Problem als nur Zuweisungen innerhalb von ifs.
Idelic
6
@sisharp Eleganz ist so im Auge des Betrachters! Ich verstehe nicht, wie in aller Welt ein Missbrauch eines Schleifenkonstrukts irgendwie als "elegant" wahrgenommen werden kann, aber vielleicht bin das nur ich.
Dasblinkenlight
38
  1. Versuchen Sie, den Code in eine separate Funktion (oder möglicherweise in mehrere) zu extrahieren. Kehren Sie dann von der Funktion zurück, wenn die Prüfung fehlschlägt.

  2. Wenn es zu eng mit dem umgebenden Code gekoppelt ist und Sie keine Möglichkeit finden, die Kopplung zu verringern, sehen Sie sich den Code nach diesem Block an. Vermutlich werden einige von der Funktion verwendete Ressourcen bereinigt. Versuchen Sie, diese Ressourcen mit einem RAII- Objekt zu verwalten. Ersetzen Sie dann jeden zwielichtigen breakdurch return(oder throw, falls dies angemessener ist) und lassen Sie den Destruktor des Objekts für Sie aufräumen.

  3. Wenn der Programmablauf (notwendigerweise) so schnörkellos ist, dass Sie wirklich einen benötigen goto, verwenden Sie diesen, anstatt ihn seltsam zu verkleiden.

  4. Wenn Sie Codierungsregeln haben, die blind verbieten goto, und Sie den Programmablauf wirklich nicht vereinfachen können, müssen Sie ihn wahrscheinlich mit Ihrem doHack verschleiern .

Mike Seymour
quelle
4
Ich gehe demütig davon aus, dass RAII zwar nützlich, aber keine Silberkugel ist. Wenn Sie kurz davor stehen, eine Konvertierungsklasse zu RAII zu schreiben, die keine andere Verwendung hat, sollten Sie auf jeden Fall besser bedient werden, wenn Sie nur die bereits erwähnten Redewendungen "Goto-End-of-the-World" verwenden.
Idoby
1
@busy_wait: In der Tat kann RAII nicht alles lösen; Deshalb hört meine Antwort nicht beim zweiten Punkt auf, sondern schlägt vor, gotoob dies wirklich eine bessere Option ist.
Mike Seymour
3
Ich stimme zu, aber ich denke, es ist eine schlechte Idee, goto-RAII-Konvertierungsklassen zu schreiben, und ich denke, es sollte explizit angegeben werden.
Idoby
@idoby Was ist mit dem Schreiben einer RAII-Vorlagenklasse und dem Instanziieren mit einer einmaligen Bereinigung, die Sie in einem Lambda benötigen
Caleth,
@Caleth klingt für mich so, als würden Sie ctors / dtors neu erfinden?
Idoby
37

TLDR : RAII , Transaktionscode (nur Ergebnisse festlegen oder , wenn dieser bereits berechnet wurde) und Ausnahmen.

Lange Antwort:

In C besteht die beste Vorgehensweise für diese Art von Code darin, dem Code ein EXIT / CLEANUP / other- Label hinzuzufügen, in dem die lokalen Ressourcen bereinigt werden und ein Fehlercode (falls vorhanden) zurückgegeben wird. Dies ist eine bewährte Methode, da der Code auf natürliche Weise in Initialisierung, Berechnung, Festschreiben und Zurückgeben aufgeteilt wird:

error_code_type c_to_refactor(result_type *r)
{
    error_code_type result = error_ok; //error_code_type/error_ok defd. elsewhere
    some_resource r1, r2; // , ...;
    if(error_ok != (result = computation1(&r1))) // Allocates local resources
        goto cleanup;
    if(error_ok != (result = computation2(&r2))) // Allocates local resources
        goto cleanup;
    // ...

    // Commit code: all operations succeeded
    *r = computed_value_n;
cleanup:
    free_resource1(r1);
    free_resource2(r2);
    return result;
}

In C ist in den meisten Codebasen der Code if(error_ok != ...und gotonormalerweise hinter einigen praktischen Makros versteckt ( RET(computation_result),ENSURE_SUCCESS(computation_result, return_code) usw.).

C ++ bietet zusätzliche Tools über C :

  • Die Bereinigungsblockfunktion kann als RAII implementiert werden. Dies bedeutet, dass Sie nicht mehr den gesamten cleanupBlock benötigen und dass der Clientcode frühe Rückgabeanweisungen hinzufügen kann.

  • Sie werfen, wann immer Sie nicht weitermachen können, und verwandeln alle if(error_ok != ...in direkte Anrufe.

Äquivalenter C ++ - Code:

result_type cpp_code()
{
    raii_resource1 r1 = computation1();
    raii_resource2 r2 = computation2();
    // ...
    return computed_value_n;
}

Dies ist eine bewährte Methode, weil:

  • Es ist explizit (das heißt, während die Fehlerbehandlung nicht explizit ist, ist der Hauptfluss des Algorithmus)

  • Es ist einfach, Client-Code zu schreiben

  • Es ist minimal

  • Es ist einfach

  • Es gibt keine sich wiederholenden Codekonstrukte

  • Es werden keine Makros verwendet

  • Es werden keine seltsamen do { ... } while(0)Konstrukte verwendet

  • Es kann mit minimalem Aufwand wiederverwendet werden (dh wenn ich den Aufruf in computation2();eine andere Funktion kopieren möchte , muss ich nicht sicherstellen, dass ich do { ... } while(0)im neuen Code ein, kein #definegoto-Wrapper-Makro und kein Bereinigungsetikett hinzufüge oder noch etwas).

utnapistim
quelle
+1. Dies ist, was ich versuche, mit RAII oder so etwas zu verwenden. shared_ptrMit einem benutzerdefinierten Deleter können viele Dinge erledigt werden. Noch einfacher mit Lambdas in C ++ 11.
Macke
Richtig, aber wenn Sie shared_ptr für Dinge verwenden, die keine Zeiger sind, sollten Sie zumindest Folgendes eingeben: namespace xyz { typedef shared_ptr<some_handle> shared_handle; shared_handle make_shared_handle(a, b, c); };In diesem Fall (mit dem make_handleFestlegen des richtigen Löschertyps bei der Konstruktion) deutet der Name des Typs nicht mehr darauf hin, dass es sich um einen Zeiger handelt .
utnapistim
21

Der Vollständigkeit halber füge ich eine Antwort hinzu. Eine Reihe anderer Antworten wies darauf hin, dass der große Bedingungsblock in eine separate Funktion aufgeteilt werden könnte. Es wurde jedoch auch mehrfach darauf hingewiesen, dass dieser Ansatz den bedingten Code vom ursprünglichen Kontext trennt. Dies ist ein Grund, warum der Sprache in C ++ 11 Lambdas hinzugefügt wurden. Die Verwendung von Lambdas wurde von anderen vorgeschlagen, es wurde jedoch keine explizite Probe bereitgestellt. Ich habe eine in diese Antwort eingefügt. Was mir auffällt, ist, dass es sich do { } while(0)in vielerlei Hinsicht sehr ähnlich anfühlt - und vielleicht bedeutet das, dass es immer noch eine gotoVerkleidung ist ...

earlier operations
...
[&]()->void {

    if (!check()) return;
    ...
    ...
    if (!check()) return;
    ...
    ...
    if (!check()) return;
    ...
    ...
}();
later operations
Peter R.
quelle
7
Für mich sieht dieser Hack schlimmer aus als der ... während des Hacks.
Michael
18

Sicher nicht die Antwort, sondern eine Antwort (der Vollständigkeit halber)

Anstatt :

do {
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
} while(0);

Sie könnten schreiben:

switch (0) {
case 0:
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
}

Dies ist immer noch ein goto in der Verkleidung, aber zumindest ist es nicht eine Schleife mehr. Das heißt, Sie müssen nicht sehr sorgfältig prüfen, ob irgendwo im Block keine weiteren versteckt sind.

Das Konstrukt ist auch so einfach, dass Sie hoffen können, dass der Compiler es wegoptimiert.

Wie von @jamesdlin vorgeschlagen, können Sie dies sogar hinter einem Makro wie verbergen

#define BLOC switch(0) case 0:

Und benutze es gerne

BLOC {
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
}

Dies ist möglich, weil die C-Sprachsyntax eine Anweisung nach einem Wechsel erwartet, keinen Block in Klammern, und Sie dieser Anweisung eine Fallbezeichnung hinzufügen können. Bis jetzt habe ich nicht den Sinn gesehen, das zuzulassen, aber in diesem speziellen Fall ist es praktisch, den Schalter hinter einem schönen Makro zu verstecken.

kriss
quelle
2
Oh, klug. Sie können es sogar hinter einem Makro wie verstecken define BLOCK switch (0) case 0:und es wie verwenden BLOCK { ... break; }.
Jamesdlin
@jamesdlin: Mir ist noch nie in den Sinn gekommen, dass es nützlich sein könnte, einen Fall vor die öffnende Klammer eines Schalters zu stellen. Aber es ist in der Tat von C erlaubt und in diesem Fall ist es praktisch, ein schönes Makro zu schreiben.
Kriss
15

Ich würde einen Ansatz empfehlen, der Mats Antwort abzüglich des Unnötigen ähnelt goto. Setzen Sie nur die bedingte Logik in die Funktion ein. Jeder Code, der immer ausgeführt wird, sollte vor oder nach dem Aufruf der Funktion im Aufrufer ausgeführt werden:

void main()
{
    //do stuff always
    func();
    //do other stuff always
}

void func()
{
    if (!condition)
        return;
    ...
    if (!other condition)
        return;
    ...
    if (!another condition)
        return;
    ... 
    if (!yet another condition)
        return;
    ...
}
Dan Bechard
quelle
3
Wenn Sie in der Mitte eine andere Ressource erwerben funcmüssen, müssen Sie eine andere Funktion (gemäß Ihrem Muster) herausrechnen. Wenn alle diese isolierten Funktionen dieselben Daten benötigen, kopieren Sie am Ende immer wieder dieselben Stapelargumente oder entscheiden sich, Ihre Argumente auf dem Heap zuzuweisen und einen Zeiger herumzugeben, ohne die grundlegendste Funktion der Sprache zu nutzen ( Funktionsargumente). Kurz gesagt, ich glaube nicht, dass diese Lösung für den schlimmsten Fall skaliert, in dem jeder Zustand überprüft wird, nachdem neue Ressourcen erworben wurden, die bereinigt werden müssen. Beachten Sie jedoch den Kommentar von Nawaz zu Lambdas.
Der
2
Ich erinnere mich nicht, dass das OP etwas über den Erwerb von Ressourcen in der Mitte seines Codeblocks gesagt hat, aber ich akzeptiere dies als machbare Anforderung. Was ist in diesem Fall falsch daran, die Ressource irgendwo auf dem Stapel zu deklarieren func()und ihrem Destruktor zu erlauben, Ressourcen freizugeben? Wenn etwas außerhalb von func()Zugriff auf dieselbe Ressource benötigt, sollte es vor dem Aufruf func()durch einen geeigneten Ressourcenmanager auf dem Heap deklariert werden .
Dan Bechard
12

Der Code-Fluss selbst ist bereits ein Code-Geruch, der in der Funktion zu viel passiert. Wenn es dafür keine direkte Lösung gibt (die Funktion ist eine allgemeine Überprüfungsfunktion), ist es möglicherweise besser , RAII zu verwenden, damit Sie zurückkehren können, anstatt zu einem Endabschnitt der Funktion zu springen.

stefaanv
quelle
11

Wenn Sie während der Ausführung keine lokalen Variablen einführen müssen, können Sie dies häufig reduzieren:

if (check()) {
  doStuff();
}  
if (stillOk()) {
  doMoreStuff();
}
if (amIStillReallyOk()) {
  doEvenMore();
}

// edit 
doThingsAtEndAndReportErrorStatus()
the_mandrill
quelle
2
Aber dann müsste jede Bedingung die vorherige enthalten, was nicht nur hässlich, sondern möglicherweise auch schlecht für die Leistung ist. Es ist besser, über diese Überprüfungen zu springen und sofort aufzuräumen, sobald wir wissen, dass wir "nicht in Ordnung" sind.
Der
Das stimmt, aber dieser Ansatz hat Vorteile, wenn am Ende einige Dinge erledigt werden müssen, damit Sie nicht vorzeitig zurückkehren möchten (siehe Bearbeiten). Wenn Sie mit Denise's Ansatz kombinieren, Bools unter den gegebenen Bedingungen zu verwenden, ist der Leistungseinbruch vernachlässigbar, es sei denn, dies ist eine sehr enge Schleife.
the_mandrill
10

Ähnlich wie die Antwort von dasblinkenlight, vermeidet jedoch die Zuordnung innerhalb der if, die von einem Code-Reviewer " repariert" werden könnte:

bool goOn = check0();
if (goOn) {
    ...
    goOn = check1();
}
if (goOn) {
    ...
    goOn = check2();
}
if (goOn) {
    ...
}

...

Ich verwende dieses Muster, wenn die Ergebnisse eines Schritts vor dem nächsten Schritt überprüft werden müssen, was sich von einer Situation unterscheidet, in der alle Überprüfungen im Voraus mit einem großen if( check1() && check2()...Muster durchgeführt werden könnten .

Denise Skidmore
quelle
Beachten Sie, dass check1 () tatsächlich PerformStep1 () sein kann, das einen Ergebniscode des Schritts zurückgibt. Dies würde die Komplexität Ihrer Prozessflussfunktion verringern.
Denise Skidmore
10

Verwenden Sie Ausnahmen. Ihr Code sieht viel sauberer aus (und Ausnahmen wurden genau für die Behandlung von Fehlern im Ausführungsablauf eines Programms erstellt). Lesen Sie zum Bereinigen von Ressourcen (Dateideskriptoren, Datenbankverbindungen usw.) den Artikel Warum bietet C ++ kein "finally" -Konstrukt? .

#include <iostream>
#include <stdexcept>   // For exception, runtime_error, out_of_range

int main () {
    try {
        if (!condition)
            throw std::runtime_error("nope.");
        ...
        if (!other condition)
            throw std::runtime_error("nope again.");
        ...
        if (!another condition)
            throw std::runtime_error("told you.");
        ...
        if (!yet another condition)
            throw std::runtime_error("OK, just forget it...");
    }
    catch (std::runtime_error &e) {
        std::cout << e.what() << std::endl;
    }
    catch (...) {
        std::cout << "Caught an unknown exception\n";
    }
    return 0;
}
Cartucho
quelle
10
"Ja wirklich?" Erstens sehe ich dort ehrlich gesagt keine Verbesserung der Lesbarkeit. VERWENDEN SIE KEINE AUSNAHMEN, UM DEN PROGRAMMFLUSS ZU STEUERN. Das ist NICHT ihr eigentlicher Zweck. Ausnahmen bringen auch erhebliche Leistungseinbußen mit sich. Der richtige Anwendungsfall für eine Ausnahme ist, wenn eine Bedingung vorliegt, dass Sie nichts dagegen tun können, z. B. der Versuch, eine Datei zu öffnen, die bereits ausschließlich von einem anderen Prozess gesperrt ist, oder eine Netzwerkverbindung fehlschlägt oder ein Aufruf einer Datenbank fehlschlägt oder ein Aufrufer übergibt einen ungültigen Parameter an eine Prozedur. Diese Art von Ding. Verwenden Sie jedoch KEINE Ausnahmen, um den Programmfluss zu steuern.
Craig
3
Ich meine, nicht um ein Rotz darüber zu sein, sondern gehen Sie zu dem Stroustrup-Artikel, auf den Sie verwiesen haben, und suchen Sie nach "Wofür sollte ich keine Ausnahmen verwenden?" Sektion. Unter anderem sagt er: "Insbesondere ist throw nicht einfach eine alternative Möglichkeit, einen Wert von einer Funktion zurückzugeben (ähnlich wie return). Dies ist langsam und verwirrt die meisten C ++ - Programmierer, die es gewohnt sind, Ausnahmen zu erkennen, die nur für Fehler verwendet werden Handhabung. Ebenso ist Werfen kein guter Weg, um aus einer Schleife herauszukommen. "
Craig
3
@Craig Alles, was Sie gezeigt haben, ist richtig, aber Sie gehen davon aus, dass das Beispielprogramm fortgesetzt werden kann, nachdem eine check()Bedingung fehlgeschlagen ist, und das ist mit Sicherheit IHRE Annahme, dass das Beispiel keinen Kontext enthält. Unter der Annahme, dass das Programm nicht fortgesetzt werden kann, ist die Verwendung von Ausnahmen der richtige Weg.
Cartucho
2
Nun, wahr genug, wenn das tatsächlich der Kontext ist. Aber eine lustige Sache über Annahmen ... ;-)
Craig
10

Für mich do{...}while(0)ist in Ordnung. Wenn Sie das nicht sehen wollendo{...}while(0) , können Sie alternative Schlüsselwörter für sie definieren.

Beispiel:

//--------SomeUtilities.hpp---------
#define BEGIN_TEST do{
#define END_TEST }while(0);

//--------SomeSourceFile.cpp--------
BEGIN_TEST
   if(!condition1) break;
   if(!condition2) break;
   if(!condition3) break;
   if(!condition4) break;
   if(!condition5) break;

   //processing code here

END_TEST

Ich denke, der Compiler wird die unnötige while(0)Bedingung in entfernendo{...}while(0) und die Pausen in einen bedingungslosen Sprung umwandeln. Sie können die Assembler-Version überprüfen, um sicherzugehen.

Die Verwendung gotoerzeugt auch saubereren Code und ist mit der Condition-Then-Jump-Logik unkompliziert. Sie können Folgendes tun:

{
   if(!condition1) goto end_blahblah;
   if(!condition2) goto end_blahblah;
   if(!condition3) goto end_blahblah;
   if(!condition4) goto end_blahblah;
   if(!condition5) goto end_blahblah;

   //processing code here

 }end_blah_blah:;  //use appropriate label here to describe...
                   //  ...the whole code inside the block.

Beachten Sie, dass das Etikett nach dem Schließen platziert wird }. Dies ist das Vermeiden eines möglichen Problems goto, da versehentlich ein Code dazwischen platziert wird, weil Sie das Etikett nicht gesehen haben. Es ist jetzt wiedo{...}while(0) ohne Bedingungscode.

Um diesen Code sauberer und verständlicher zu machen, können Sie Folgendes tun:

//--------SomeUtilities.hpp---------
#define BEGIN_TEST {
#define END_TEST(_test_label_) }_test_label_:;
#define FAILED(_test_label_) goto _test_label_

//--------SomeSourceFile.cpp--------
BEGIN_TEST
   if(!condition1) FAILED(NormalizeData);
   if(!condition2) FAILED(NormalizeData);
   if(!condition3) FAILED(NormalizeData);
   if(!condition4) FAILED(NormalizeData);
   if(!condition5) FAILED(NormalizeData);

END_TEST(NormalizeData)

Mit dieser Option können Sie verschachtelte Blöcke erstellen und angeben, wo Sie beenden / herausspringen möchten.

//--------SomeUtilities.hpp---------
#define BEGIN_TEST {
#define END_TEST(_test_label_) }_test_label_:;
#define FAILED(_test_label_) goto _test_label_

//--------SomeSourceFile.cpp--------
BEGIN_TEST
   if(!condition1) FAILED(NormalizeData);
   if(!condition2) FAILED(NormalizeData);

   BEGIN_TEST
      if(!conditionAA) FAILED(DecryptBlah);
      if(!conditionBB) FAILED(NormalizeData);   //Jump out to the outmost block
      if(!conditionCC) FAILED(DecryptBlah);

      // --We can now decrypt and do other stuffs.

   END_TEST(DecryptBlah)

   if(!condition3) FAILED(NormalizeData);
   if(!condition4) FAILED(NormalizeData);

   // --other code here

   BEGIN_TEST
      if(!conditionA) FAILED(TrimSpaces);
      if(!conditionB) FAILED(TrimSpaces);
      if(!conditionC) FAILED(NormalizeData);   //Jump out to the outmost block
      if(!conditionD) FAILED(TrimSpaces);

      // --We can now trim completely or do other stuffs.

   END_TEST(TrimSpaces)

   // --Other code here...

   if(!condition5) FAILED(NormalizeData);

   //Ok, we got here. We can now process what we need to process.

END_TEST(NormalizeData)

Spaghetti-Code ist nicht die Schuld des gotoProgrammierers. Sie können weiterhin Spaghetti-Code ohne Verwendung erstellen goto.

acegs
quelle
10
Ich würde mich dafür entscheiden goto, die Sprachsyntax mit dem Präprozessor millionenfach zu erweitern.
Christian
2
"Um diesen Code sauberer und verständlicher zu machen, können Sie [LOADS_OF_WEIRD_MACROS verwenden]" : wird nicht berechnet.
underscore_d
8

Dies ist ein bekanntes und aus funktionaler Programmierperspektive gut gelöstes Problem - die vielleicht Monade.

Als Antwort auf den Kommentar, den ich unten erhalten habe, habe ich meine Einführung hier bearbeitet: Sie finden ausführliche Informationen zur Implementierung von C ++ - Monaden an verschiedenen Stellen, mit denen Sie das erreichen können, was Rotsor vorschlägt. Es dauert eine Weile, um Monaden zu groken. Stattdessen werde ich hier einen schnellen monadenähnlichen Mechanismus für "arme Männer" vorschlagen, für den Sie nur Boost :: optional kennen müssen.

Richten Sie Ihre Berechnungsschritte wie folgt ein:

boost::optional<EnabledContext> enabled(boost::optional<Context> context);
boost::optional<EnergisedContext> energised(boost::optional<EnabledContext> context);

Jeder Rechenschritt kann offensichtlich so etwas wie eine Rückgabe ausführen, boost::nonewenn die Option, die er erhalten hat, leer ist. Also zum Beispiel:

struct Context { std::string coordinates_filename; /* ... */ };

struct EnabledContext { int x; int y; int z; /* ... */ };

boost::optional<EnabledContext> enabled(boost::optional<Context> c) {
   if (!c) return boost::none; // this line becomes implicit if going the whole hog with monads
   if (!exists((*c).coordinates_filename)) return boost::none; // return none when any error is encountered.
   EnabledContext ec;
   std::ifstream file_in((*c).coordinates_filename.c_str());
   file_in >> ec.x >> ec.y >> ec.z;
   return boost::optional<EnabledContext>(ec); // All ok. Return non-empty value.
}

Dann verkette sie:

Context context("planet_surface.txt", ...); // Close over all needed bits and pieces

boost::optional<EnergisedContext> result(energised(enabled(context)));
if (result) { // A single level "if" statement
    // do work on *result
} else {
    // error
}

Das Schöne daran ist, dass Sie für jeden Rechenschritt klar definierte Komponententests schreiben können. Auch der Aufruf liest sich wie einfaches Englisch (wie es normalerweise beim funktionalen Stil der Fall ist).

Wenn Sie sich nicht für Unveränderlichkeit interessieren und es bequemer ist, jedes Mal dasselbe Objekt zurückzugeben, wenn Sie mit shared_ptr oder ähnlichem eine Variation finden könnten.

Benedikt
quelle
3
Dieser Code hat die unerwünschte Eigenschaft, jede der einzelnen Funktionen zu zwingen, den Fehler der vorherigen Funktion zu behandeln, wodurch das Monaden-Idiom nicht richtig verwendet wird (wobei monadische Effekte, in diesem Fall Fehler, implizit behandelt werden sollen). Dazu müssen Sie optional<EnabledContext> enabled(Context); optional<EnergisedContext> energised(EnabledContext);stattdessen die monadische Kompositionsoperation ('bind') anstelle der Funktionsanwendung verwenden.
Rotsor
Vielen Dank. Sie haben Recht - so machen Sie es richtig. Ich wollte nicht zu viel in meine Antwort schreiben, um das zu erklären (daher der Begriff "armer Mann", der darauf hindeuten sollte, dass ich hier nicht das ganze Schwein gehen würde).
Benedikt
7

Wie wäre es, wenn Sie die if-Anweisungen in eine zusätzliche Funktion verschieben, um ein numerisches oder enum-Ergebnis zu erhalten?

int ConditionCode (void) {
   if (condition1)
      return 1;
   if (condition2)
      return 2;
   ...
   return 0;
}


void MyFunc (void) {
   switch (ConditionCode ()) {
      case 1:
         ...
         break;

      case 2:
         ...
         break;

      ...

      default:
         ...
         break;
   }
}
karx11erx
quelle
Das ist schön, wenn möglich, aber viel weniger allgemein als die hier gestellte Frage. Jede Bedingung kann von dem Code abhängen, der nach dem letzten Entzweigungstest ausgeführt wurde.
Kriss
Das Problem hierbei ist, dass Sie Ursache und Konsequenz trennen. Das heißt, Sie trennen Code, der sich auf dieselbe Bedingungsnummer bezieht, und dies kann eine Quelle für weitere Fehler sein.
Riga
@kriss: Nun, die ConditionCode () -Funktion könnte angepasst werden, um dies zu berücksichtigen. Der entscheidende Punkt dieser Funktion ist, dass Sie return <Ergebnis> für einen sauberen Exit verwenden können, sobald eine endgültige Bedingung berechnet wurde. Und genau das schafft hier strukturelle Klarheit.
Karx11erx
@Riga: Imo das sind völlig akademische Einwände. Ich finde, dass C ++ mit jeder neuen Version komplexer, kryptischer und unlesbarer wird. Ich sehe kein Problem mit einer kleinen Hilfsfunktion, die komplexe Bedingungen gut strukturiert bewertet, um die Zielfunktion direkt darunter besser lesbar zu machen.
Karx11erx
1
@ karx11erx meine Einwände sind praktisch und basieren auf meiner Erfahrung. Dieses Muster ist unabhängig von C ++ 11 oder einer anderen Sprache schlecht. Wenn Sie Schwierigkeiten mit Sprachkonstrukten haben, mit denen Sie eine gute Architektur schreiben können, ist dies kein Sprachproblem.
Riga
5

So etwas vielleicht

#define EVER ;;

for(EVER)
{
    if(!check()) break;
}

oder Ausnahmen verwenden

try
{
    for(;;)
        if(!check()) throw 1;
}
catch()
{
}

Mit Ausnahmen können Sie auch Daten übergeben.

Liftarn
quelle
10
Bitte machen Sie keine cleveren Dinge wie Ihre Definition von jemals, sie erschweren normalerweise das Lesen von Code für andere Entwickler. Ich habe jemanden gesehen, der Case als break; case in einer Header-Datei definiert und in einem Schalter in einer cpp-Datei verwendet hat, sodass sich andere stundenlang fragen, warum der Wechsel zwischen Case-Anweisungen unterbrochen wird. Grrr ...
Michael
5
Und wenn Sie Makros benennen, sollten Sie sie wie Makros aussehen lassen (dh in Großbuchstaben). Andernfalls jemand, der zufällig eine Variable / Funktion / Typ / etc. Benennt. benannt everwird sehr unglücklich sein ...
Jamesdlin
5

Ich bin nicht besonders in der Art und Weise mit breakoderreturn in einem solchen Fall. Angesichts der Tatsache, dass wir normalerweise in einer solchen Situation sind, ist dies normalerweise eine vergleichsweise lange Methode.

Wenn wir mehrere Austrittspunkte haben, kann dies zu Schwierigkeiten führen, wenn wir wissen möchten, was dazu führt, dass bestimmte Logik ausgeführt wird: Normalerweise gehen wir einfach weiter nach oben, um diese Logik einzuschließen, und die Kriterien dieser umschließenden Blöcke geben uns Auskunft darüber Lage:

Beispielsweise,

if (conditionA) {
    ....
    if (conditionB) {
        ....
        if (conditionC) {
            myLogic();
        }
    }
}

Wenn Sie sich die umschließenden Blöcke ansehen, können Sie leicht herausfinden, dass dies myLogic()nur dann geschieht, wennconditionA and conditionB and conditionC es wahr ist.

Es wird viel weniger sichtbar, wenn es frühe Renditen gibt:

if (conditionA) {
    ....
    if (!conditionB) {
        return;
    }
    if (!conditionD) {
        return;
    }
    if (conditionC) {
        myLogic();
    }
}

Wir können nicht mehr von oben navigieren myLogic() nach umschließenden Block betrachten, um den Zustand herauszufinden.

Es gibt verschiedene Problemumgehungen, die ich verwendet habe. Hier ist einer von ihnen:

if (conditionA) {
    isA = true;
    ....
}

if (isA && conditionB) {
    isB = true;
    ...
}

if (isB && conditionC) {
    isC = true;
    myLogic();
}

(Natürlich ist es willkommen, dieselbe Variable zu verwenden, um alle zu ersetzen isA isB isC .)

Ein solcher Ansatz gibt dem Leser zumindest Code, der myLogic()ausgeführt wird, wenn isB && conditionC. Der Leser erhält einen Hinweis, dass er weiter nachschlagen muss, was dazu führt, dass isB wahr ist.

Adrian Shum
quelle
3
typedef bool (*Checker)();

Checker * checkers[]={
 &checker0,&checker1,.....,&checkerN,NULL
};

bool checker1(){
  if(condition){
    .....
    .....
    return true;
  }
  return false;
}

bool checker2(){
  if(condition){
    .....
    .....
    return true;
  }
  return false;
}

......

void doCheck(){
  Checker ** checker = checkers;
  while( *checker && (*checker)())
    checker++;
}

Wie wär es damit?

Sniperbat
quelle
Das Wenn ist veraltet, nur return condition;, sonst denke ich, dass dies gut wartbar ist.
SpaceTrucker
2

Ein weiteres Muster, das nützlich ist, wenn Sie je nach Fehlerort unterschiedliche Bereinigungsschritte benötigen:

    private ResultCode DoEverything()
    {
        ResultCode processResult = ResultCode.FAILURE;
        if (DoStep1() != ResultCode.SUCCESSFUL)
        {
            Step1FailureCleanup();
        }
        else if (DoStep2() != ResultCode.SUCCESSFUL)
        {
            Step2FailureCleanup();
            processResult = ResultCode.SPECIFIC_FAILURE;
        }
        else if (DoStep3() != ResultCode.SUCCESSFUL)
        {
            Step3FailureCleanup();
        }
        ...
        else
        {
            processResult = ResultCode.SUCCESSFUL;
        }
        return processResult;
    }
Denise Skidmore
quelle
2

Ich bin kein C ++ - Programmierer, daher schreibe ich hier keinen Code, aber bisher hat niemand eine objektorientierte Lösung erwähnt. Also hier ist meine Vermutung dazu:

Verfügen Sie über eine generische Schnittstelle, die eine Methode zur Bewertung einer einzelnen Bedingung bereitstellt. Jetzt können Sie eine Liste von Implementierungen dieser Bedingungen in Ihrem Objekt verwenden, die die betreffende Methode enthalten. Sie durchlaufen die Liste und bewerten jede Bedingung, wobei Sie möglicherweise früh ausbrechen, wenn eine fehlschlägt.

Das Gute ist, dass ein solches Design sehr gut am Open / Closed-Prinzip festhält , da Sie während der Initialisierung des Objekts, das die betreffende Methode enthält, leicht neue Bedingungen hinzufügen können. Sie können der Schnittstelle sogar eine zweite Methode hinzufügen, wobei die Methode zur Bedingungsbewertung eine Beschreibung der Bedingung zurückgibt. Dies kann für selbstdokumentierende Systeme verwendet werden.

Der Nachteil ist jedoch, dass aufgrund der Verwendung von mehr Objekten und der Iteration über die Liste etwas mehr Overhead erforderlich ist.

SpaceTrucker
quelle
Könnten Sie ein Beispiel in einer anderen Sprache hinzufügen? Ich denke, diese Frage gilt für viele Sprachen, obwohl sie speziell zu C ++ gestellt wurde.
Denise Skidmore
1

So mache ich es.

void func() {
  if (!check()) return;
  ...
  ...

  if (!check()) return;
  ...
  ...

  if (!check()) return;
  ...
  ...
}
Vadoff
quelle
1

Zunächst ein kurzes Beispiel, um zu zeigen, warum dies gotokeine gute Lösung für C ++ ist:

struct Bar {
    Bar();
};

extern bool check();

void foo()
{
    if (!check())
       goto out;

    Bar x;

    out:
}

Versuchen Sie, dies in eine Objektdatei zu kompilieren und sehen Sie, was passiert. Versuchen Sie dann das entsprechende do+ break+while(0) .

Das war eine Seite. Der Hauptpunkt folgt.

Diese kleinen Stücke von Code erfordern oft einige Art Bereinigung sollte die gesamte Funktion fehlschlagen. Diese Bereinigungen möchten normalerweise in der entgegengesetzten Reihenfolge von den Blöcken selbst erfolgen, wenn Sie die teilweise abgeschlossene Berechnung "abwickeln".

Eine Option, um diese Semantik zu erhalten, ist RAII ; siehe @ utnapistims Antwort. C ++ garantiert, dass automatische Destruktoren in der entgegengesetzten Reihenfolge zu Konstruktoren ausgeführt werden, was natürlich ein "Abwickeln" liefert.

Dafür sind jedoch viele RAII-Klassen erforderlich. Manchmal ist es einfacher, nur den Stapel zu verwenden:

bool calc1()
{
    if (!check())
        return false;

    // ... Do stuff1 here ...

    if (!calc2()) {
        // ... Undo stuff1 here ...
        return false;
    }

    return true;
}

bool calc2()
{
    if (!check())
        return false;

    // ... Do stuff2 here ...

    if (!calc3()) {
        // ... Undo stuff2 here ...
        return false;
    }

    return true;
}

...und so weiter. Dies ist einfach zu prüfen, da der "Rückgängig" -Code neben dem "Do" -Code steht. Einfache Prüfung ist gut. Es macht auch den Kontrollfluss sehr klar. Es ist auch ein nützliches Muster für C.

Es kann erforderlich sein, dass die calcFunktionen viele Argumente annehmen, aber das ist normalerweise kein Problem, wenn Ihre Klassen / Strukturen einen guten Zusammenhalt haben. (Das heißt, Dinge, die zusammen gehören, leben in einem einzelnen Objekt, sodass diese Funktionen Zeiger oder Verweise auf eine kleine Anzahl von Objekten aufnehmen und dennoch viele nützliche Arbeiten ausführen können.)

Nemo
quelle
Es ist sehr einfach, den Bereinigungspfad zu überwachen, aber möglicherweise nicht so einfach, den goldenen Pfad zu verfolgen. Aber insgesamt denke ich, dass so etwas ein konsistentes Bereinigungsmuster fördert.
Denise Skidmore
0

Wenn Ihr Code einen langen Block von if..else if..else-Anweisungen enthält, können Sie versuchen, den gesamten Block mit Hilfe von Functorsoder neu zu schreiben function pointers. Es ist vielleicht nicht immer die richtige Lösung, aber es ist ziemlich oft.

http://www.cprogramming.com/tutorial/functors-function-objects-in-c++.html

Hal
quelle
Im Prinzip ist dies möglich (und nicht schlecht), aber mit expliziten Funktionsobjekten oder -zeigern wird der Codefluss viel zu stark gestört. OTOH die hier äquivalente Verwendung von Lambdas oder gewöhnlichen benannten Funktionen ist eine gute Praxis, effizient und liest sich gut.
links um den
0

Ich bin erstaunt über die Anzahl der verschiedenen Antworten, die hier präsentiert werden. Aber schließlich habe ich in dem Code, den ich ändern muss (dh diesen do-while(0)Hack oder irgendetwas entfernen ), etwas anderes getan als die hier erwähnten Antworten, und ich bin verwirrt, warum niemand dies gedacht hat. Folgendes habe ich getan:

Anfangscode:

do {

    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
    if(!check()) break;
    ...
    ...
} while(0);

finishingUpStuff.

Jetzt:

finish(params)
{
  ...
  ...
}

if(!check()){
    finish(params);    
    return;
}
...
...
if(!check()){
    finish(params);    
    return;
}
...
...
if(!check()){
    finish(params);    
    return;
}
...
...

Was hier gemacht wurde, ist, dass das Finishing-Zeug in einer Funktion isoliert wurde und die Dinge plötzlich so einfach und sauber geworden sind!

Ich fand diese Lösung erwähnenswert und stellte sie hier zur Verfügung.

Sankalp
quelle
0

Konsolidieren Sie es in einer ifAussage:

if(
    condition
    && other_condition
    && another_condition
    && yet_another_condition
    && ...
) {
        if (final_cond){
            //Do stuff
        } else {
            //Do other stuff
        }
}

Dies ist das Muster, das in Sprachen wie Java verwendet wird, in denen das Schlüsselwort goto entfernt wurde.

Tyzoid
quelle
2
Das funktioniert nur, wenn Sie zwischen den Bedingungstests nichts tun müssen. (Nun, ich nehme an, Sie könnten das Do-of-Zeug in einigen Funktionsaufrufen verstecken, die von den Bedingungstests durchgeführt wurden, aber das könnte ein bisschen verschleiert werden, wenn Sie es zu viel getan haben)
Jeremy Friesner
@JeremyFriesner Eigentlich könnten Sie das Zwischenmaterial als separate boolesche Funktionen ausführen, die immer als ausgewertet werden true. Die Kurzschlussauswertung würde garantieren, dass Sie niemals Zwischenprodukte ausführen, für die nicht alle erforderlichen Tests bestanden wurden.
AJMansfield
@AJMansfield Ja, darauf habe ich in meinem zweiten Satz Bezug genommen ... aber ich bin mir nicht sicher, ob dies eine Verbesserung der Codequalität bedeuten würde.
Jeremy Friesner
@JeremyFriesner Nichts hindert Sie daran, die Bedingungen so zu schreiben (/*do other stuff*/, /*next condition*/), dass Sie sie sogar gut formatieren können. Erwarten Sie nur nicht, dass die Leute es mögen. Aber ehrlich gesagt zeigt dies nur, dass es ein Fehler für Java war, diese gotoAussage
auslaufen zu lassen
@ JeremyFriesner Ich nahm an, dass dies Boolesche Werte waren. Wenn Funktionen innerhalb jeder Bedingung ausgeführt werden sollen, gibt es eine bessere Möglichkeit, damit umzugehen.
Tyzoid
0

Wenn Sie für alle Fehler denselben Fehlerbehandler verwenden und jeder Schritt einen Bool zurückgibt, der den Erfolg anzeigt:

if(
    DoSomething() &&
    DoSomethingElse() &&
    DoAThirdThing() )
{
    // do good condition action
}
else
{
    // handle error
}

(Ähnlich wie bei Tyzoids Antwort, aber Bedingungen sind die Aktionen, und das && verhindert, dass nach dem ersten Fehler zusätzliche Aktionen ausgeführt werden.)

Denise Skidmore
quelle
0

Warum wurde die Markierungsmethode nicht beantwortet? Sie wird seit Ewigkeiten verwendet.

//you can use something like this (pseudocode)
long var = 0;
if(condition)  flag a bit in var
if(condition)  flag another bit in var
if(condition)  flag another bit in var
............
if(var == certain number) {
Do the required task
}
Fennekin
quelle