Forum: PC-Programmierung C++ If else oder return?


von Torben (Gast)


Lesenswert?

Hallo,

wie ist eure Haltung zu if else oder return in meiner Methode?

Was ist besser bezüglich Geschwindigkeit und Lesbarkeit oder gibt es 
hierfür definierte Vorgaben?
1
void PowerSupply::SetOutputOnOff(bool onoff)
2
{
3
    const QString SetOutput("OUT");
4
5
    if(m_busy)
6
        return;
7
8
    if(onoff)
9
    {
10
        serialPortWriter->write(SetOutput.toUtf8() + " 1\r");
11
        return;
12
    }
13
    serialPortWriter->write(SetOutput.toUtf8() + " 0\r");
14
}

von Michael G. (mjgraf)


Lesenswert?

Lesbarkeit ist Geschmackssache. Persönlich finde ich returns bei 
Abbruchbedingungen besser lesbar, als den gesamten Rest der Funktion in 
einen else-Block zu setzen. Man muss sich aber dessen bewusst sein, dass 
eventueller clean-up-code am Ende der Funktion dann nicht erreicht wird, 
um bspw. allozierten Speicher wieder freizugeben.

Hinsichtlich der Geschwindigkeit sollte ein optimierender Compiler in 
beiden Fällen identischen Code erzeugen.

von A. S. (Gast)


Lesenswert?

Torben schrieb:
> wie ist eure Haltung zu if else oder return in meiner Methode?
da lohnt die Frage nicht, siehe unten

> Was ist besser bezüglich Geschwindigkeit und Lesbarkeit oder gibt es
> hierfür definierte Vorgaben?
Geschwindigkeit sollte in der Frage nie eine Rolle spielen.
Lesbarkeit und Robustheit ist das Kriterum.
Sobald Du anfängst, Code doppelt zu schreiben, weil Du eine Regel 
einhälst, stimmt was mit der Regel nicht.
1
void PowerSupply::SetOutputOnOff(bool onoff)
2
{
3
    const QString SetOutput("OUT");
4
5
    if(!m_busy)
6
    { 
7
        if(onoff)
8
        {
9
            serialPortWriter->write(SetOutput.toUtf8() + " 1\r");
10
        }
11
        else
12
        {
13
            serialPortWriter->write(SetOutput.toUtf8() + " 0\r");
14
        }
15
    }
16
}

Da lohnt kein Streit für. Wobei die beiden writes für mich untereinander 
gehören:
1
void PowerSupply::SetOutputOnOff(bool onoff)
2
{
3
const QString SetOutput("OUT");
4
5
    if(!m_busy)
6
    { 
7
        if(onoff) {serialPortWriter->write(SetOutput.toUtf8() + " 1\r");}
8
        else      {serialPortWriter->write(SetOutput.toUtf8() + " 0\r");}
9
    }
10
}
So erkennt man den Unterschied sofort. Ich teile den unbedingten 
Zeilen-Fetisch nicht.

von Dussel (Gast)


Lesenswert?

Unabhängig von dem Beispiel benutze ich lieber return. Das finde ich 
viel übersichtlicher, als eine oder sogar mehrere if-Ebenen einzubauen.

Manche argumentieren, dass dadurch der Code zu Spaghetticode würde, aber 
das finde ich gar nicht. Mit return wird die Funktion an der Stelle 
verlassen und zum Aufruf zurückgesprungen. Da ist ganz klar, was 
passiert.

von Sven B. (scummos)


Lesenswert?

Kommt auf den logischen Zusammenhang an.

Das busy-Return -- klar.

Das if für den 1-Fall und dann ein return statt else für den 0-Fall 
finde ich komisch.

Ich würde diesen Fall vermutlich so oder so ähnlich schreiben:
1
void PowerSupply::SetOutputOnOff(bool on)
2
{
3
    if (m_busy)
4
        return;
5
6
    auto cmd = QString("OUT %1\n").arg(on ? "1" : "0");
7
    serialPortWriter->write(cmd.toUtf8());
8
}

: Bearbeitet durch User
von mh (Gast)


Lesenswert?

A. S. schrieb:
> Sobald Du anfängst, Code doppelt zu schreiben, weil Du eine Regel
> einhälst, stimmt was mit der Regel nicht.
> [...]
Du meinst also eigentlich sowas wie
1
void PowerSupply::SetOutputOnOff(bool onoff)
2
{
3
  const QString SetOutput("OUT");
4
  const char* lineEnd = onoff ? " 1\r" : " 0\r";
5
  if(!m_busy)
6
  {
7
    serialPortWriter->write(SetOutput.toUtf8() + lineEnd);
8
  }
9
}
um das doppelte write zu vermeiden?

von Achim H. (anymouse)


Lesenswert?

Die Version von Sven B.gefällt mir gut.

Das Problem mit dem return mittendrin ist, dass man es vielleicht 
übersieht, und noch etwas ans Ende der Methode setzt, was eigentlich 
immer durchlaufen werden soll, aber aufgrund ders vorzeitigen returns 
bedingt deaktiviert wird.

Aber "multiple returns" ist sowieso eher ein Glaubenskrieg ...

von P. S. (namnyef)


Lesenswert?

Ich bin eher ein Anhänger davon, dass eine Funktion nur einen 
Aussprungpunkt haben sollte. Und dieser sollte am Ende der Funktion 
sein. Das wird für sicherheitsrelevanten Code ja meist auch in Normen 
und Coding-Standards (z.B. MISRA) gefordert.
Bei sehr trivialen Funktionen dürfen es von mir aus auch mehrere 
returns sein. Die MISRA-Regel, die einen "single point of exit" fordert, 
ist ja auch nur "advisory".

Die ursprüngliche Variante des Threaderstellers finde ich schon mal sehr 
suboptimal. Da gefällt mir die erste Variante von A. S. 1000x besser. Da 
ist einfach auf den ersten Blick sonnenklar was passiert.

Man muss halt immer damit rechnen, dass der Code in ferner Zukunft von 
jemand anderem oder auch von einem selbst wieder angefasst wird. Und bei 
mehreren returns passiert schnell mal Murks.

von Sven B. (scummos)


Lesenswert?

P. S. schrieb:
> Ich bin eher ein Anhänger davon, dass eine Funktion nur einen
> Aussprungpunkt haben sollte. Und dieser sollte am Ende der Funktion
> sein. Das wird für sicherheitsrelevanten Code ja meist auch in Normen
> und Coding-Standards (z.B. MISRA) gefordert.

Ich nicht, ich finde es meist eher unlesbarer als die Alternative.

Das MISRA-Zeug und generell generische Sicherheits/bla-Standards ist 
gefühlt oft eher dafür designt, den schlimmsten Scheiß bestmöglich 
abzuwenden. Hat ja sicherlich auch seine Berechtigung. Als generelle 
Code-Richtlinie würde ich das aber nicht einführen wollen.

Aber ich will dir deine Meinung nicht absprechen, ist, wie schon gesagt 
wurde, ein Glaubenskrieg und es gibt nicht die eine Wahrheit. Man kann 
sicherlich mit jeder Variante lesbaren und unlesbaren Code schreiben.

von Sven B. (scummos)


Lesenswert?

Noch was, wenn du ein Byte Array willst, solltest du einfach direkt 
QByteArray benutzen und nicht den Umweg über den (intern 
UTF16-codierten) QString gehen und denn dann in UTF8 umwandeln. Das 
bringt weder Klarheit noch Performance.

von Irgendwer (Gast)


Lesenswert?

P. S. schrieb:
> Ich bin eher ein Anhänger davon, dass eine Funktion nur einen
> Aussprungpunkt haben sollte. Und dieser sollte am Ende der Funktion
> sein.
Da schließe ich mich voll und ganz an.
Hat gelegentlich auch verteile wenn man beim debuggen sich erstmal nur 
den Rückgabewert anschauen will und nicht gleich dutzende Breakpoints 
setzen will damit man auch jeden möglichen Aussprungpunkt zu erwischen.


P. S. schrieb:
> Die ursprüngliche Variante des Threaderstellers finde ich schon mal sehr
> suboptimal.

Dazu kommt noch das durch das "void" ein mittendrin verstecktest 
"return" fast schon "unerwartet" ist (wenn auch syntaktisch hier völlig 
richtig verwendet).
Bei so kurzen Funktionen geht das ja alles noch, aber sowie die länger 
werden übersieht man so ein verstecktes return auch mal ganz schnell.

von Vincent H. (vinci)


Lesenswert?

Irgendwer schrieb:
> Dazu kommt noch das durch das "void" ein mittendrin verstecktest
> "return" fast schon "unerwartet" ist (wenn auch syntaktisch hier völlig
> richtig verwendet).
> Bei so kurzen Funktionen geht das ja alles noch, aber sowie die länger
> werden übersieht man so ein verstecktes return auch mal ganz schnell.


Wenn die Funktion nicht mehr auf den Bildschirm passt, dann is es Zeit 
für Refactoring.

Mir persönlich wäre bei einem Code-Review das if vollkommen egal. Da 
gibts wesentlich wichtigere Fragen. Z.B. wieso is die Methode nicht 
const?

von A.S. (Gast)


Lesenswert?

mh schrieb:
> Du meinst also eigentlich sowas wie

Ja. Nur kostet ein zusätzliches token auch wieder temporären 
Hirnspeicher, dass ich es hier noch nicht tun würde.

von Rolf M. (rmagnus)


Lesenswert?

Sven B. schrieb:
> Noch was, wenn du ein Byte Array willst, solltest du einfach direkt
> QByteArray benutzen und nicht den Umweg über den (intern
> UTF16-codierten) QString gehen und denn dann in UTF8 umwandeln. Das
> bringt weder Klarheit noch Performance.

Oder mann nimmt QStringLiteral oder macht es wenigstens static, damit 
nicht bei jedem Funktionsaufruf neu der Speicher dynamisch allokiert und 
dann "OUT" nach UCS4 konvertiert werden muss, nur um dann nochmal neu zu 
allokieren, damit man einen weiteren String erstellt in UTF8, den man 
dann wieder in einen neuen String kopiert, wo noch eine 0 oder 1 
dahinter hängt.
Wenn einen die Performance interessiert (die aber hier wahrscheinlich eh 
kaum eine Rolle spielen wird), ist die Frage, wo da das if steht und ob 
man mehrere returns nutzt, vollkommen unerheblich im Vergleich zu diesen 
ganzen Speicherallokationen. Da würde ich mir eher das ganze Gehampel 
mit den QStrings sparen und einfach schreiben:

void PowerSupply::SetOutputOnOff(bool onoff)
{
    if (!m_busy)
        serialPortWriter->write(onoff ? "OUT 1\r" : "OUT 0\r");
}

: Bearbeitet durch User
von Ich mag Enten (Gast)


Lesenswert?

Das hardgecodede Lineending ist natürlich nicht ganz portable...

von Rolf M. (rmagnus)


Lesenswert?

Ich mag Enten schrieb:
> Das hardgecodede Lineending ist natürlich nicht ganz portable...

Es geht hier ja ganz offensichtlich um die Ausgabe auf einer seriellen 
Schnittstelle. Da muss man das Protokoll umsetzen, was die Gegenstelle 
erwartet.

von Sven B. (scummos)


Lesenswert?

Vincent H. schrieb:
> Mir persönlich wäre bei einem Code-Review das if vollkommen egal. Da
> gibts wesentlich wichtigere Fragen. Z.B. wieso is die Methode nicht
> const?

Weil sie auf einen Serial Port schreibt und damit den Zustand des 
Systems verändert?

von A.S. (Gast)


Lesenswert?

Dass die Funktion so wenig Sinn macht, ist denke ich jedem klar. 
Trotzdem kann man den Teilaspekt return oder nicht ja daran ausführen.

von Rolf M. (rmagnus)


Lesenswert?

A.S. schrieb:
> Dass die Funktion so wenig Sinn macht, ist denke ich jedem klar.

Naja, ob es dem TE klar ist, ist erstmal nicht ersichtlich.

> Trotzdem kann man den Teilaspekt return oder nicht ja daran ausführen.

Was das if (m_busy) betrifft: Ich finde es in Ordnung, wenn bei einer 
Prüfung der Vorbedingungen zu Beginn bei nicht-Erfüllung gleich ein 
return gemacht wird. Das finde ich besser, als den ganzen eigentlichen 
Funktionscode komplett in ein if (oder gar mehrere) zu stecken.
Ansonsten sollte man etwas vorsichtig damit sein, return-Anweisung 
überall in der Funktion zu verteilen, weil das durchaus der 
Übersichtlichkeit schaden kann.
Was das if(onoff) betrifft: Ich würde hier die zweite write-Anweisung in 
einen else-Block schreiben. Hier ist ja kein komplett unterschiedlicher 
Funktionsablauf gegeben, sondern es wird in beiden Fällen das gleiche 
gemacht, nur mit einem etwas unterschiedlichen Text. Da halte ich ein 
vorzeitiges return für nicht gerechtfertigt und würde auch alleine schon 
aus Symmetrie-Gründen den else-Zweig stattdessen verwenden.

: Bearbeitet durch User
von Torben (Gast)


Lesenswert?

Viele nette Anregungen sind dazugekommen. Danke.

Die Befehle kommen später alle in eine static const lookuptable und 
extra h Datei.

Die Schreibweisen von Sven B. und Rolf M. gefallen mir am besten.

>Dass die Funktion so wenig Sinn macht, ist denke ich jedem klar.
>Trotzdem kann man den Teilaspekt return oder nicht ja daran ausführen.

Wieso sollte es kein Sinn machen?

von Torben (Gast)


Lesenswert?

Bzgl. dem EOL:

Bekommt natürlich auch eine variable, damit bei einem anderen Netzteil 
mit anderem Protokoll auch funktioniert.

von Rolf M. (rmagnus)


Lesenswert?

Torben schrieb:
> Bzgl. dem EOL:
>
> Bekommt natürlich auch eine variable, damit bei einem anderen Netzteil
> mit anderem Protokoll auch funktioniert.

Da ist die Wahrscheinlichkeit aber groß, dass nicht nur das EOL anders 
ist. Dafür kann man dann Polymorphie verwenden und einfach eine zweite 
Netzteil-Klasse implementieren, die von einer gemeinsamen Basisklasse 
ableitet.

von A. S. (Gast)


Lesenswert?

Torben schrieb:
> Wieso sollte es kein Sinn machen?

darum:

Torben schrieb:
> Die Befehle kommen später alle in eine static const lookuptable und
> extra h Datei.

Du wirst, sobald Du 5 Funktionen davon geschrieben hast, die 
Gemeinsamkeiten sehen und zusammenfassen. Sobald Du dann mehr Erfahrung 
mit Stringverarbeitung hast, bleibt quasi jeweils nur ein einzeiler 
übrig. Oder was Array-basiertes, oder ... oder...

von no goto (Gast)


Lesenswert?

Jedes return, das nicht am Ende der Funktion steht, entspricht einem 
goto (END:)!

Möge jeder der Befürworter hier selbst entscheiden, wie er zum goto 
steht.

;-)

von A. S. (Gast)


Lesenswert?

no goto schrieb:
> Jedes return, das nicht am Ende der Funktion steht, entspricht einem
> goto (END:)!

genauso wie ein break. Das vermeide ich auch konsequent bei 
switch/case...

von Rolf M. (rmagnus)


Lesenswert?

no goto schrieb:
> Jedes return, das nicht am Ende der Funktion steht, entspricht einem
> goto (END:)!

Das gilt in C++ (und diversen anderen Sprachen) auch für jede Exception, 
denn die beenden die Funktion auch vorzeitig, genau wie beim return.

von Nur so eine Idee (Gast)


Lesenswert?

Theoretisch sollten in C++ die Exceptions und das "goto end" keine 
Probleme ergeben.

Im Gegensatz zu C übernehmen in C++ die Destruktoren der lokalen 
Variablen die Aufräumarbeiten. In C mussten wir uns noch selbst um 
free() oder close(file) kümmern. Wenn so ein "return" Probleme ergibt, 
sind die Destruktoren falsch konzipiert.

von loeti2 (Gast)


Lesenswert?

If there is nothing to do, getta outa here ;-)

Das meint ich bevorzuge ein early return immer wenn die 
Eingangsparameter
oder ein bestimmter Zustand keine Abarbeitung des Funktionsrumpfes 
zulassen.

Deshalb für mich nichts schlimmes in der TO-Variante.

von Rufus Τ. F. (rufus) Benutzerseite


Lesenswert?

A. S. schrieb:
> genauso wie ein break. Das vermeide ich auch konsequent bei
> switch/case...

Zeig doch bitte mal ein Beispiel. Für mich hört sich das so an, als 
sollte man sich mit Grausen davon abwenden.

von nicht“Gast“ (Gast)


Lesenswert?

Rufus Τ. F. schrieb:
> Zeig doch bitte mal ein Beispiel. Für mich hört sich das so an, als
> sollte man sich mit Grausen davon abwenden.

Bring mal deinen Ironiedetektor in die Werkstatt. Der scheint kaputt zu 
sein. ;)

von no goto (Gast)


Lesenswert?

A. S. schrieb:
> genauso wie ein break. Das vermeide ich auch konsequent bei
> switch/case...

Fachlich bist du ne Null Nummer, wenn du jede Funktion mit einer switch 
case_ enden läßt. :-P

von Rufus Τ. F. (rufus) Benutzerseite


Lesenswert?

nicht“Gast“ schrieb:
> Bring mal deinen Ironiedetektor in die Werkstatt.

Bei den bizarren Vorstellungen, die hier manche für einen 
empfehlenswerten Programmierstil halten, bin ich vorsichtig, hier gleich 
"Ironie" zu attestieren. Wir haben hier im Forum mehrere ernsthafte 
Verfechter von "magic numbers" im Quelltext, und die Vehemenz, mit der 
auch andere unerwartete Meinungen verteidigt werden, lässt beliebige 
Abweichungen von tradierten Erwartungshaltungen zu.

von no goto (Gast)


Lesenswert?

Rolf M. schrieb:
> Das gilt in C++ (und diversen anderen Sprachen) auch für jede Exception,
> denn die beenden die Funktion auch vorzeitig, genau wie beim return.

Schon cool, wenn hier ein selbst ernannter Superprofi  exzeptionell und 
goto gleichwertig sieht. ???

von Nur so eine Idee (Gast)


Lesenswert?

> gleichwertig sieht

Da ist was dran.

Exceptions, goto und return ergeben das selbe Problem. Du müsst höllisch 
aufpassen. Kannst leicht bei einigen Pfaden die Aufräumarbeiten 
vergessen.

Im Beispiel ganz oben klappt es. Der Destruktor des QString kümmert sich 
um alle Aufräumarbeiten. Und der C++ Compiler ruft den Destruktor sowohl 
bei Exceptions, als auch bei return auf. Meist auch wenn ein goto den 
Scope verlässt.

Wenn die Klasse aber eine explizite close() Methode verlangt, hast du in 
allen 3 Fällen das selbe Problem.

von Sven B. (scummos)


Lesenswert?

Nur so eine Idee schrieb:
> Wenn die Klasse aber eine explizite close() Methode verlangt, hast du in
> allen 3 Fällen das selbe Problem.

Die Idee von C++ ist aber, dass sie das nicht tut. Deshalb halte ich das 
"early returns sind BÖSE"-Theater auch in C++ nicht für sinnvoll.

von DPA (Gast)


Lesenswert?

Ich programmiere gerne in C, und finde es oft ebenfalls übersichtlicher, 
bei der Fehlerbehandlung und ähnlichem bei dem die Funktion logisch am 
"Ende" ist, mit return und goto zu arbeiten. Nichts ist schlimmer als 
unübersichtliche mehrfach Verschachtelungen, spätestens ab der 5. Ebene 
muss das übersichtlicher gemacht werden, sofern möglich. Wobei, oft 
braucht man dann nicht goto, sondern Unterfunktionen. Es muss halt 
jeweils einfach sinn machen, an der Stelle ein "Stop, alles liegen 
lassen, gehe zum Notfallexit." hinzuschreiben.

Normalerweise prüfe ich meistens Dinge wie Argumente usw. Wenn vorher 
noch keine locks gesetzt wurden und auch sonst nichts gemacht wurde, und 
irgendwas nicht gut ist, gibt das erstmal ein return. Manchmal auch ein 
Goto ans Ende wenn das in der Funktionen an anderen stellen noch gemacht 
wird.

Die Fehlerbehandlung mit goto ist relativ simpel. Man packt ans ende der 
Funktion label, die das schon gemachte in umgekehrter Richtung wieder 
aufräumen. Beispiel:
1
struct bla* create_bla(int x){
2
  if(x < 0)
3
    goto error;
4
5
  struct bla* bla = malloc(sizeof(struct bla));
6
  if(!bla)
7
    goto error;
8
9
  if(init_bla_bla(bla) == -1)
10
    goto error_after_malloc;
11
12
  if(do_bla_bla(bla, x) == -1)
13
    goto error_after_init_bla_bla;
14
15
  return bla;
16
17
error_after_init_bla_bla:
18
  cleanup_bla_bla(bla);
19
error_after_malloc:
20
  free(bla);
21
error:
22
  return 0;
23
}

Klar, in C++ hat man dafür Exceptions. Die wären wirklich praktisch, und 
es gibt auch Implementationen in C. Einziges "Problem" mit diesen ist 
halt, dass die aus einer Funktion heraus zu viel springen können, wenn 
man mal vergisst, eine zu behandeln.

Natürlich ist sowas nicht empfehlenswert:

Torben schrieb:
> if(onoff)
>     {
>         serialPortWriter->write(SetOutput.toUtf8() + " 1\r");
>         return;
>     }
>     serialPortWriter->write(SetOutput.toUtf8() + " 0\r");

Da ist ja entweder das eine, oder das andere, und dann noch weiteres, 
momentan das Funktionsende. Das Asymetrisch zu machen, und im einen Fall 
direkt zu beenden statt da normal zum ende zu gehen, macht deshalb rein 
logisch keinen Sinn. Ich würde sogar soweit gehen zu sagen, dass es 
falsch ist.

von Dirk K. (merciless)


Lesenswert?

Das erste if ist aus Clean Code-Sicht ok,
das 2. fragwürdig.

Generell bin ich dafür, erst die Randbedingungen
zu erklären (ifs mit evtl. return) und dann den
Mainflow am Stück schreiben.

Und: Der Vergleich der vorzeitigen returns mit
goto ist Bullshit.

merciless

von Nur so eine Idee (Gast)


Lesenswert?

> Die Fehlerbehandlung mit goto ist relativ simpel.

Wenn das alle so machen würden, könnte das durchaus eine gute Strategie 
sein. Aber es gibt sowieso schon viel zu viele unterschiedliche 
Strategien.

Lieber ein Programm aus 10 gleichartigen mittelmässigen Komponenten 
zusammen stellen, als aus 10 genialen, aber inkompatiblen.

von Torben (Gast)


Lesenswert?

Ich denke das erste Return entspricht der Guard Clause. Mittendrin die 
returns sind nicht schön zumindest wenn ich die Antworten von den Usern 
zähle.

Falls jemand über den Post stolpert.

https://medium.com/@scadge/if-statements-design-guard-clauses-might-be-all-you-need-67219a1a981a

von Nur so eine Idee (Gast)


Lesenswert?

> sind nicht schön

Lisp-Programmierer vertreten da die entgegengesetzte Ansicht. Die finden 
Zuweisungen nicht schön und verlassen die Funktion an zig verschiedenen 
Stellen, sobald sie einen Return-Wert berechnet haben.

Wie ist das entstanden? Entscheiden sich Leute, die Returns nicht schon 
finden für C? Oder sind die C-Programmierer so oft auf die Schnauze 
gefallen, dass sie Returns nicht mehr als schön empfinden?

von DPA (Gast)


Lesenswert?

Ich hab nie lisp Programiert, aber einiges darüber gehört. Ist dort die 
Idee nicht, dass eine Funktion immer pure ist, also man nie irgend einen 
globalen Zustand ändert und deshalb nie Nebeneffekte hat? Dann kann ein 
return dort nämlich nie schaden. Und ist das dort nicht teils sogar 
notwendig, früher ein return zu haben, weil dort Funktionen oft rekursiv 
aufeinander und auf sich selbst aufbauend definiert sind, und sonst gar 
kein Ergebnis hätten oder ewig laufen würden? Das ist halt eine andere 
Art des Programmierens, die für andere Aufgaben gut geeignet sind. Das 
kann man zu einem gewissen Grad für gewisse Dinge in C Anwenden, aber 
dort kann man das halt auch falsch machen. C ist halt keine so "pure" 
Sprache wie Lisp ;)

von no goto (Gast)


Lesenswert?

Dirk K. schrieb:
> Und: Der Vergleich der vorzeitigen returns mit
> goto ist Bullshit.

Nö, genauso ist es: vorzeitiges return ist wie ein goto ENDE:

von Nur so eine Idee (Gast)


Lesenswert?

> Nö, genauso ist es: vorzeitiges return ist wie ein goto ENDE:

Zumindest intern erzeugt ein C++ Compiler aus den returns den selben 
Code wie DPAs goto Beispiel.

Die selbe interne Technik, nur eine andere Abstraktion darüber gelegt. 
Somit lassen sich die beiden Abstraktionen miteinander vergleichen.

Dirk K. kann nicht behaupten, der Vergleich an sich wäre Bullshit. Er 
kann höchsten behaupten: wer sich nicht seiner religiösen Überzeugung 
anschliesst, sei ein Ochse.

von A. S. (Gast)


Lesenswert?

Nur so eine Idee schrieb:
> Dirk K. kann nicht behaupten, der Vergleich an sich wäre Bullshit. Er
> kann höchsten behaupten: wer sich nicht seiner religiösen Überzeugung
> anschliesst, sei ein Ochse.

Nein, Bullshit ist der Sprung, der hier gemacht wird:

no goto schrieb:
> jedes return, das nicht am Ende der Funktion steht, entspricht einem
> goto (END:)!
>
> Möge jeder der Befürworter hier selbst entscheiden, wie er zum goto
> steht.

Genauso gut könnte er (richtig) sagen: "Die schließende '}' einer 
while-Schleife ist wie ein goto zurück an den Anfang."

Sein zweiter Satz wäre dann genauso irreführend:
> Möge jeder der Befürworter hier selbst entscheiden, wie er zum goto
> steht.

von mh (Gast)


Lesenswert?

A. S. schrieb:
> Genauso gut könnte er (richtig) sagen: "Die schließende '}' einer
> while-Schleife ist wie ein goto zurück an den Anfang."

Das ist falsch. Die '}' "beendet" mindestens auch noch einen Scope.

von Oh nein (Gast)


Lesenswert?

Die Verfechter von "nur ein return pro Funktion" mögen sich mal diesen 
völlig MISRA C++ konformen Code anschauen:

https://vcis-gitlab.f4e.europa.eu/aneto/MARTe2/blob/master/Source/Core/BareMetal/L5GAMs/MemoryMapBroker.cpp
https://vcis-gitlab.f4e.europa.eu/aneto/MARTe2/blob/master/Source/Core/BareMetal/L5GAMs/GAMDataSource.cpp

immer schön das (ret) mitgeschleift, obwohl schon mglw. ganz am Anfang 
feststeht, dass nichts weiter damit zu machen ist. Es darf ja nur ein 
return geben...

Das eigentliche Problem ist, dass jemand cyclomatic complexity zur 
ultimativen Metrik für Codequalität/-komplexität hochstilisiert hat. Ist 
es aber nicht.

von no goto (Gast)


Lesenswert?

DPA schrieb:
> Beispiel:
> struct bla* create_bla(int x){
>   if(x < 0)
>     goto error;
>
>   struct bla* bla = malloc(sizeof(struct bla));
>   if(!bla)
>     goto error;
>
>   if(init_bla_bla(bla) == -1)
>     goto error_after_malloc;
>
>   if(do_bla_bla(bla, x) == -1)
>     goto error_after_init_bla_bla;
>
>   return bla;
>
> error_after_init_bla_bla:
>   cleanup_bla_bla(bla);
> error_after_malloc:
>   free(bla);
> error:
>   return 0;
> }

Erinnert mich an BASIC Spaghetti Code.

So geht das in C
1
struct bla* create_bla(int x){
2
  struct bla* blabla;
3
  if(x < 0) {
4
    blabla= NULL;
5
  }
6
  else {
7
    blabla= malloc(sizeof(struct bla));
8
    if(blabla) {
9
      if(init_bla_bla(blabla) == -1) {
10
        free(blabla);
11
        blabla= NULL;
12
      }
13
      else if(do_bla_bla(blabla, x) == -1) {
14
        cleanup_bla_bla(blabla);
15
        free(blabla);
16
        blabla= NULL;
17
      }
18
      // no else: blabla is on heap
19
    }
20
    // no else: blabla is NULL;
21
  }
22
  return blabla;
23
}

von no goto (Gast)


Lesenswert?

Oh nein schrieb:
> immer schön das (ret) mitgeschleift, obwohl schon mglw. ganz am Anfang
> feststeht, dass nichts weiter damit zu machen ist.

if ... else ... !!!

von Vincent H. (vinci)


Lesenswert?

Oh nein schrieb:
> Das eigentliche Problem ist, dass jemand cyclomatic complexity zur
> ultimativen Metrik für Codequalität/-komplexität hochstilisiert hat. Ist
> es aber nicht.

Ich stimme dir zwar zu, aber deine Begründung ist falsch. Die cyclomatic 
complexity sinkt mit jedem return weil das den aktuellen Pfad sofort 
schließt.

Hier ein recht aktueller kurzer Talk dazu:
https://youtu.be/6gbcU2VNqAI

von Rolf M. (rmagnus)


Lesenswert?

no goto schrieb:
> So geht das in C

Sieht furchtbar aus. Die Verschachtelung macht es schlechter lesbar, und 
die Code-Duplikation macht es fehleranfällig. Da würde ich die Version 
mit goto definitiv bevorzugen.

von DPA (Gast)


Lesenswert?

no goto schrieb:
> So geht das in C

Sehr schön, jetzt bist du schon auf der 3. Ebenen und hast die Stellen 
wo ein Free nötig ist verdoppelt, und die wo du was auf 0 setzen musst 
verunendlich facht (0x -> 3x). Entsprechend sind auch die stellen, wo 
die vergessen werden können, vervielfacht. Aber es muss ja besser sein, 
wenn es kein böses goto drinn hat...

von Dirk K. (merciless)


Lesenswert?

Zitat von 
https://softwareengineering.stackexchange.com/questions/18454/should-i-return-from-a-function-early-or-use-an-if-statement:
1
public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
2
{
3
    int retval = SUCCESS;
4
    if (someCondition)
5
    {
6
        if (name != null && name != "")
7
        {
8
            if (value != 0)
9
            {
10
                if (perms.allow(name)
11
                {
12
                    // Do Something
13
                }
14
                else
15
                {
16
                    reval = PERM_DENY;
17
                }
18
            }
19
            else
20
            {
21
                retval = BAD_VALUE;
22
            }
23
        }
24
        else
25
        {
26
            retval = BAD_NAME;
27
        }
28
    }
29
    else
30
    {
31
        retval = BAD_COND;
32
    }
33
    return retval;
34
}

vs.
1
public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
2
{
3
    if (!someCondition)
4
        return BAD_COND;
5
6
    if (name == null || name == "")
7
        return BAD_NAME;
8
9
    if (value == 0)
10
        return BAD_VALUE;
11
12
    if (!perms.allow(name))
13
        return PERM_DENY;
14
15
    // Do something
16
    return SUCCESS;
17
}

Möge jeder für sich entscheiden...

merciless

: Bearbeitet durch User
von Eric B. (beric)


Lesenswert?

Dirk K. schrieb:
> Möge jeder für sich entscheiden...

Dann so ;-)
1
public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
2
{
3
    int ret = SUCCESS;
4
5
    if (!cond1) {
6
        ret = BAD_COND;
7
    } else if (name == null || name == "") {
8
        ret = BAD_NAME;
9
    } else if (value == 0) {
10
        ret = BAD_VALUE;
11
    } else if (!perms.allow(name)) {
12
        ret = PERM_DENY;
13
    } else {
14
      // Do something
15
    }
16
17
    return ret;
18
}
 oder sogar:
1
public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
2
{
3
    int ret = SUCCESS;
4
5
    if      (!cond1)                     { ret = BAD_COND;  } 
6
    else if (name == null || name == "") { ret = BAD_NAME;  } 
7
    else if (value == 0)                 { ret = BAD_VALUE; } 
8
    else if (!perms.allow(name))         { ret = PERM_DENY; } 
9
    else {
10
      // Do something
11
    }
12
13
    return ret;
14
}

: Bearbeitet durch User
von Yalu X. (yalu) (Moderator)


Lesenswert?

Dirk K. schrieb:
> Möge jeder für sich entscheiden...

Ich bin zwar kein großer Freund von Mehrfach-Returns, aber in diesem
konkreten Beispiel würde ich wie du die zweite Variante (mit den vielen
Returns) bevorzugen.

Anders sieht es aus, wenn am Ende der Funktion auch im Fehlerfall noch
weiterer Code ausgeführt werden soll. Dann versagt, die Methode mit den
Mehrfach-Returns. Dennoch gibt es IMHO bessere Methoden als die Variante
1 (mit der starken Verschachtelung).

Eine Möglichkeit ist die Verwendung von Gotos:

1
public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
2
{
3
    int retval = SUCCESS;
4
    if (!someCondition) {
5
        retval = BAD_COND;
6
        goto cont;
7
    }
8
9
    if (name == null || name == "") {
10
        retval = BAD_NAME;
11
        goto cont;
12
    }
13
14
    if (value == 0) {
15
        retval = BAD_VALUE;
16
        goto cont;
17
    }
18
19
    if (!perms.allow(name)) {
20
        retval = PERM_DENY;
21
        goto cont;
22
    }
23
24
    else
25
        // do something on success
26
27
cont:
28
    // do something in any case
29
30
    return retval;
31
}


Eine andere verwendet statt der Gotos eine If-ElseIf-Kette (Eric war
schneller, da hatte ich diesen Teil aber schon geschrieben):

1
public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
2
{
3
    int retval = SUCCESS;
4
5
    if (!someCondition)
6
        retval = BAD_COND;
7
8
    else if (name == null || name == "")
9
        retval = BAD_NAME;
10
11
    else if (value == 0)
12
        retval = BAD_VALUE;
13
14
    else if (!perms.allow(name))
15
        retval = PERM_DENY;
16
17
    else
18
        // do something on success
19
20
    // do something in any case
21
22
    return retval;
23
}

Hier würde ich tendenziell der zweiten Variante (ohne Gotos) den Vorzug
geben.


Nur so eine Idee schrieb:
> Lisp-Programmierer vertreten da die entgegengesetzte Ansicht. Die finden
> Zuweisungen nicht schön und verlassen die Funktion an zig verschiedenen
> Stellen, sobald sie einen Return-Wert berechnet haben.

Man muss dabei beachten, dass das return in Common-Lisp eher dem break
als dem return in C entspricht, da es wie das break immer nur einen
Block beendet. Während das break in C aber nur Schleifen und Switch-
Anwesungen beendet, kann das return in Lisp beliebige Blöcke beenden.
Außerdem kann dem return in Lisp ein Wert mitgegeben werden, der dann
zum Wert des beendeten Blocks wird.

So kann man ziemlich elegant (ein echter Lisp-Programmierer, der ich
nicht bin, bekommt es sicher noch besser hin ;-)) eine Serie von
Bedingungen (test1 bis test4) abfragen und entsprechende Fehlercodes
setzen. Da die Returns nur den umschließenden Block, nicht aber die
Funktion beenden, kann auch im Fehlerfall weiterer Code ausgeführt
werden.

1
(defun f ()
2
  (prog1
3
    (block nil
4
      (cond ((not test1) (return 'TEST1-FAILED))
5
            ((not test2) (return 'TEST2-FAILED))
6
            ((not test3) (return 'TEST3-FAILED))
7
            ((not test4) (return 'TEST4-FAILED)))
8
      ; ...
9
      ; do something on success
10
      ; ...
11
      'SUCCESS)
12
    ; ...
13
    ; do something in any case
14
    ; ...
15
  ))

von no goto (Gast)


Lesenswert?

Beitrag "Re: C++ If else oder return?"
Na seht ihr, geht doch. ?

Und noch mehr Zuspruch gegen goto (und damit mehrfache return).
Beitrag "Re: C++ If else oder return?"

von Nur so eine Idee (Gast)


Lesenswert?

> "Die schließende '}' einer while-Schleife ist wie ein goto
> zurück an den Anfang."

Ja klar.

Angefangen hatte das mit Dijkstras Vorschlag, wir sollen nur 3 Arten von 
Sprüngen verwenden.
- Zurück an den Anfang eines Blockes.
- Einen Block überspringen
- Einen Block als Unterprogramm aufrufen

Dann entwickelten verschiedene Leute Programmiersprachen, die nur diese 
3 Arten erlauben.

Inzwischen haben wir erkannt, das dürfen wir nicht so dogmatisch sehen. 
Es gibt Situationen, in denen es besser ist, wir weichen von Dijkstras 
Vorschrift ab.

In diesen Thread passiert nun etwas vollkommen perverses.

Diejenigen, die behaupten, wir sollen Dijkstra Vorschrift nicht so 
dogmatisch befolgen, sind selbst vollkommen dogmatische 
Fundamentalisten. Wollen nur ihre eigene Abweichung von der reinen Lehre 
erlauben.

von Timm R. (Firma: privatfrickler.de) (treinisch)


Lesenswert?

Hey,

Torben schrieb:
> Ich denke das erste Return entspricht der Guard Clause. Mittendrin die
> returns sind nicht schön zumindest wenn ich die Antworten von den Usern
> zähle.

jepp.

Die recht junge Programmiersprache Swift, die extrem von C++ beeinflusst 
ist, hat genau davon inspiriert deswegen auch ein ausdrückliches "guard" 
statement.

So in der Art:
1
    guard x > 0 else {return}

Diese guard statements bieten zwar noch ein paar Extras, aber sie sind 
im Prinzip nichts anderes, als genau die guard clauses, die in C++ 
mittlerweile absolut üblich und verbreitet sind.

Guard clauses sind in c++ idiomatisch, leicht lesbar und übersichtlich, 
es spricht nichts dagegen, sie zu verwenden und einiges dafür.

vlg
 Timm

: Bearbeitet durch User
von Tom (Gast)


Lesenswert?

Eine Frage zu den "guard clauses":

Ich habe das Gefühl, dass hier beim Funktionsaufruf ein "Haufen" blind 
über den Zaun geschmissen wird und der andere sich über die 
Sinnhaftigkeit der Argumente kümmern darf.
Sollte man nicht vorher prüfen, ob die übergebenen Argumente/Parameter 
plausibel sind?

von Achim H. (anymouse)


Lesenswert?

Tom schrieb:
> Sollte man nicht vorher prüfen, ob die übergebenen Argumente/Parameter
> plausibel sind?

Nicht unbedingt. Vielleicht gibt es ja auch Fälle, wo die Argumente 
plausibel sind, aber keinen größeren Aufwand machen.

Beispiel "Sortiere eine Liste":

* Liste ist Nullpointer --> Sortiertung braucht nichts zu machen
* Liste ist leer --> Sortiertung braucht nichts zu machen
* Liste hat 1 Element --> Sortiertung braucht nichts zu machen

Erst ab mehreren Elementen muss man den Sortieralgorithmus aufrufen.

von DPA (Gast)


Lesenswert?

Tom schrieb:
> Ich habe das Gefühl, dass hier beim Funktionsaufruf ein "Haufen" blind
> über den Zaun geschmissen wird und der andere sich über die
> Sinnhaftigkeit der Argumente kümmern darf.
> Sollte man nicht vorher prüfen, ob die übergebenen Argumente/Parameter
> plausibel sind?

Das kommt ganz drauf an. Wenn bei aufrufen von verschiedenen Orten aus 
andere Randbedingungen gelten, muss man diese ausserhalb checken. Die 
Sachen, die aber immer geprüft werden können oder müssen, packt man 
besser in die Funktion, damit man später keine Überraschungen erlebt, 
und keine Fehler macht weil man es an X stellen vor dem Prüfen wieder 
checken muss. Besonders wenn man eine Aufgabe in einer Funktion 
verallgemeinert, kann sowas extrem hilfreich sein. Das ist aber manchmal 
sehr schwierig, weil man dann manchmal alle Verwendungsarten kennen 
muss, um die Funktionsargumente/API passend für alle Anwendungsfälle zu 
wählen. Sobald man wieder komplexere dinge umrechnen muss, ist die 
Abstraktion nicht mehr ideal und sollte eventuell angepasst werden.

Als ein gelungenes Beispiel von mir würde ich mal meine Funktion zum 
ändern der Cursorposition in meinem Terminal Multiplexer/Emulator 
anführen:
https://github.com/Daniel-Abrecht/libttymultiplex/blob/c519098e0b3c4b0442267d6505ea6195fc36c5e8/src/pane.c#L291
Alle Cursormanipulationen erfolgen nur über diese Funktion, die von 
ungefähr 20 Orten aus verwendet wird. (Grösstenteils von escape 
sequenzen): 
https://github.com/Daniel-Abrecht/libttymultiplex/search?p=1&q=tym_i_pane_set_cursor_position&unscoped_q=tym_i_pane_set_cursor_position
Damit werden fast alle Fehler vermieden, die bei manuellen Checks an 
jedem der Orte sonst auftreten könnten. Andererseits war es extrem 
schwer, eine gute & allgemeingültige Abstraktion zu finden und sauber zu 
implementieren. Und bei so einer Zentralen Funktion sind auch Fehler, 
besonders im Design, sehr viel schlimmer und schwerer zu beheben. Wenn 
man es hinkriegt überwiegen die Vorteile aber.

von Vincent H. (vinci)


Lesenswert?

Tom schrieb:
> Eine Frage zu den "guard clauses":
>
> Ich habe das Gefühl, dass hier beim Funktionsaufruf ein "Haufen" blind
> über den Zaun geschmissen wird und der andere sich über die
> Sinnhaftigkeit der Argumente kümmern darf.
> Sollte man nicht vorher prüfen, ob die übergebenen Argumente/Parameter
> plausibel sind?

"guard clauses" werden in C++20 als "contracts" kommen:
https://en.cppreference.com/w/cpp/language/attributes/contract

Natürlich sollte man auch bisher so programmiert haben als gäbe es diese 
bereits. Sprich, pre-/post usw. conditions sollten Teil der 
Dokumentation sein und oder via assertions überprüft werden.

von Tom (Gast)


Lesenswert?

Danke für eure hilfreichen Antworten :-)

von Oliver S. (oliverso)


Lesenswert?

Vincent H. schrieb:
> "guard clauses" werden in C++20 als "contracts" kommen:
> https://en.cppreference.com/w/cpp/language/attributes/contract

Das sind aber statische Compilezeit-Checks, vor allem an Typen, während 
es hier im Thread eher um Laufzeitchecks von Wertebereichen geht.

Oliver

von mh (Gast)


Lesenswert?

Oliver S. schrieb:
> Vincent H. schrieb:
>> "guard clauses" werden in C++20 als "contracts" kommen:
>> https://en.cppreference.com/w/cpp/language/attributes/contract
>
> Das sind aber statische Compilezeit-Checks, vor allem an Typen, während
> es hier im Thread eher um Laufzeitchecks von Wertebereichen geht.
>
> Oliver

Sicher, dass du die Seite gelesen hast? Gleich am Anfang steht:
1
Explanation
2
1) Defines a precondition, i.e., the function's expectation of its arguments and/or the state of other objects upon entry into the function. ...

In den Beispielen steht sowas
1
expects: i > 0

Das liest sich nicht sehr statisch.

von Sven B. (scummos)


Lesenswert?

Oliver S. schrieb:
> Vincent H. schrieb:
>> "guard clauses" werden in C++20 als "contracts" kommen:
>> https://en.cppreference.com/w/cpp/language/attributes/contract
>
> Das sind aber statische Compilezeit-Checks

Sind es nicht.

von Oliver S. (oliverso)


Lesenswert?

Ups, ich hatte „concepts“ gelesen. Freud‘sche Fehlleistung.

Olivet

Bitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.