mikrocontroller.net

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


Announcement: there is an English version of this forum on EmbDev.net. Posts you create there will be displayed on Mikrocontroller.net and EmbDev.net.
Autor: Torben (Gast)
Datum:

Bewertung
0 lesenswert
nicht 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?
void PowerSupply::SetOutputOnOff(bool onoff)
{
    const QString SetOutput("OUT");

    if(m_busy)
        return;

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

Autor: Michael G. (mjgraf)
Datum:

Bewertung
3 lesenswert
nicht 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.

Autor: A. S. (achs)
Datum:

Bewertung
0 lesenswert
nicht 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.
void PowerSupply::SetOutputOnOff(bool onoff)
{
    const QString SetOutput("OUT");

    if(!m_busy)
    { 
        if(onoff)
        {
            serialPortWriter->write(SetOutput.toUtf8() + " 1\r");
        }
        else
        {
            serialPortWriter->write(SetOutput.toUtf8() + " 0\r");
        }
    }
}

Da lohnt kein Streit für. Wobei die beiden writes für mich untereinander 
gehören:
void PowerSupply::SetOutputOnOff(bool onoff)
{
const QString SetOutput("OUT");

    if(!m_busy)
    { 
        if(onoff) {serialPortWriter->write(SetOutput.toUtf8() + " 1\r");}
        else      {serialPortWriter->write(SetOutput.toUtf8() + " 0\r");}
    }
}
So erkennt man den Unterschied sofort. Ich teile den unbedingten 
Zeilen-Fetisch nicht.

Autor: Dussel (Gast)
Datum:

Bewertung
2 lesenswert
nicht 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.

Autor: Sven B. (scummos)
Datum:

Bewertung
2 lesenswert
nicht 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:
void PowerSupply::SetOutputOnOff(bool on)
{
    if (m_busy)
        return;

    auto cmd = QString("OUT %1\n").arg(on ? "1" : "0");
    serialPortWriter->write(cmd.toUtf8());
}

: Bearbeitet durch User
Autor: mh (Gast)
Datum:

Bewertung
0 lesenswert
nicht 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
void PowerSupply::SetOutputOnOff(bool onoff)
{
  const QString SetOutput("OUT");
  const char* lineEnd = onoff ? " 1\r" : " 0\r";
  if(!m_busy)
  {
    serialPortWriter->write(SetOutput.toUtf8() + lineEnd);
  }
}
um das doppelte write zu vermeiden?

Autor: Achim H. (anymouse)
Datum:

Bewertung
2 lesenswert
nicht 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 ...

Autor: P. S. (namnyef)
Datum:

Bewertung
0 lesenswert
nicht 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.

Autor: Sven B. (scummos)
Datum:

Bewertung
1 lesenswert
nicht 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.

Autor: Sven B. (scummos)
Datum:

Bewertung
0 lesenswert
nicht 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.

Autor: Irgendwer (Gast)
Datum:

Bewertung
-1 lesenswert
nicht 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.

Autor: Vincent H. (vinci)
Datum:

Bewertung
0 lesenswert
nicht 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?

Autor: A.S. (Gast)
Datum:

Bewertung
0 lesenswert
nicht 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.

Autor: Rolf M. (rmagnus)
Datum:

Bewertung
1 lesenswert
nicht 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
Autor: Ich mag Enten (Gast)
Datum:

Bewertung
-1 lesenswert
nicht lesenswert
Das hardgecodede Lineending ist natürlich nicht ganz portable...

Autor: Rolf M. (rmagnus)
Datum:

Bewertung
0 lesenswert
nicht 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.

Autor: Sven B. (scummos)
Datum:

Bewertung
0 lesenswert
nicht 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?

Autor: A.S. (Gast)
Datum:

Bewertung
0 lesenswert
nicht 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.

Autor: Rolf M. (rmagnus)
Datum:

Bewertung
3 lesenswert
nicht 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
Autor: Torben (Gast)
Datum:

Bewertung
0 lesenswert
nicht 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?

Autor: Torben (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Bzgl. dem EOL:

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

Autor: Rolf M. (rmagnus)
Datum:

Bewertung
0 lesenswert
nicht 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.

Autor: A. S. (achs)
Datum:

Bewertung
0 lesenswert
nicht 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...

Autor: no goto (Gast)
Datum:

Bewertung
-3 lesenswert
nicht 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.

;-)

Autor: A. S. (achs)
Datum:

Bewertung
2 lesenswert
nicht 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...

Autor: Rolf M. (rmagnus)
Datum:

Bewertung
2 lesenswert
nicht 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.

Autor: Nur so eine Idee (Gast)
Datum:

Bewertung
4 lesenswert
nicht 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.

Autor: loeti2 (Gast)
Datum:

Bewertung
5 lesenswert
nicht 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.

Autor: Rufus Τ. F. (rufus) (Moderator) Benutzerseite
Datum:

Bewertung
0 lesenswert
nicht 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.

Autor: nicht“Gast“ (Gast)
Datum:

Bewertung
3 lesenswert
nicht 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. ;)

Autor: no goto (Gast)
Datum:

Bewertung
-5 lesenswert
nicht 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

Autor: Rufus Τ. F. (rufus) (Moderator) Benutzerseite
Datum:

Bewertung
5 lesenswert
nicht 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.

Autor: no goto (Gast)
Datum:

Bewertung
-6 lesenswert
nicht 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. 💩💩💩

Autor: Nur so eine Idee (Gast)
Datum:

Bewertung
1 lesenswert
nicht 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.

Autor: Sven B. (scummos)
Datum:

Bewertung
5 lesenswert
nicht 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.

Autor: DPA (Gast)
Datum:

Bewertung
2 lesenswert
nicht 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:
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;
}

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.

Autor: Dirk K. (merciless)
Datum:

Bewertung
4 lesenswert
nicht 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

Autor: Nur so eine Idee (Gast)
Datum:

Bewertung
0 lesenswert
nicht 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.

Autor: Torben (Gast)
Datum:

Bewertung
1 lesenswert
nicht 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

Autor: Nur so eine Idee (Gast)
Datum:

Bewertung
1 lesenswert
nicht 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?

Autor: DPA (Gast)
Datum:

Bewertung
0 lesenswert
nicht 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 ;)

Autor: no goto (Gast)
Datum:

Bewertung
-6 lesenswert
nicht 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:

Autor: Nur so eine Idee (Gast)
Datum:

Bewertung
-1 lesenswert
nicht 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.

Autor: A. S. (achs)
Datum:

Bewertung
3 lesenswert
nicht 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.

Autor: mh (Gast)
Datum:

Bewertung
0 lesenswert
nicht 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.

Autor: Oh nein (Gast)
Datum:

Bewertung
4 lesenswert
nicht 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.

Autor: no goto (Gast)
Datum:

Bewertung
-3 lesenswert
nicht 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
struct bla* create_bla(int x){
  struct bla* blabla;
  if(x < 0) {
    blabla= NULL;
  }
  else {
    blabla= malloc(sizeof(struct bla));
    if(blabla) {
      if(init_bla_bla(blabla) == -1) {
        free(blabla);
        blabla= NULL;
      }
      else if(do_bla_bla(blabla, x) == -1) {
        cleanup_bla_bla(blabla);
        free(blabla);
        blabla= NULL;
      }
      // no else: blabla is on heap
    }
    // no else: blabla is NULL;
  }
  return blabla;
}

Autor: no goto (Gast)
Datum:

Bewertung
-2 lesenswert
nicht 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 ... !!!

Autor: Vincent H. (vinci)
Datum:

Bewertung
0 lesenswert
nicht 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:
Youtube-Video "Simplifying control flow - Zbigniew Skowron - code::dive 2018"

Autor: Rolf M. (rmagnus)
Datum:

Bewertung
4 lesenswert
nicht 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.

Autor: DPA (Gast)
Datum:

Bewertung
2 lesenswert
nicht 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...

Autor: Dirk K. (merciless)
Datum:

Bewertung
2 lesenswert
nicht lesenswert
Zitat von 
https://softwareengineering.stackexchange.com/questions/18454/should-i-return-from-a-function-early-or-use-an-if-statement:
public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
{
    int retval = SUCCESS;
    if (someCondition)
    {
        if (name != null && name != "")
        {
            if (value != 0)
            {
                if (perms.allow(name)
                {
                    // Do Something
                }
                else
                {
                    reval = PERM_DENY;
                }
            }
            else
            {
                retval = BAD_VALUE;
            }
        }
        else
        {
            retval = BAD_NAME;
        }
    }
    else
    {
        retval = BAD_COND;
    }
    return retval;
}

vs.
public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
{
    if (!someCondition)
        return BAD_COND;

    if (name == null || name == "")
        return BAD_NAME;

    if (value == 0)
        return BAD_VALUE;

    if (!perms.allow(name))
        return PERM_DENY;

    // Do something
    return SUCCESS;
}

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

merciless

: Bearbeitet durch User
Autor: Eric B. (beric)
Datum:

Bewertung
-1 lesenswert
nicht lesenswert
Dirk K. schrieb:
> Möge jeder für sich entscheiden...

Dann so ;-)
public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
{
    int ret = SUCCESS;

    if (!cond1) {
        ret = BAD_COND;
    } else if (name == null || name == "") {
        ret = BAD_NAME;
    } else if (value == 0) {
        ret = BAD_VALUE;
    } else if (!perms.allow(name)) {
        ret = PERM_DENY;
    } else {
      // Do something
    }

    return ret;
}
 oder sogar:
public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
{
    int ret = SUCCESS;

    if      (!cond1)                     { ret = BAD_COND;  } 
    else if (name == null || name == "") { ret = BAD_NAME;  } 
    else if (value == 0)                 { ret = BAD_VALUE; } 
    else if (!perms.allow(name))         { ret = PERM_DENY; } 
    else {
      // Do something
    }

    return ret;
}

: Bearbeitet durch User
Autor: Yalu X. (yalu) (Moderator)
Datum:

Bewertung
0 lesenswert
nicht 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:

public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
{
    int retval = SUCCESS;
    if (!someCondition) {
        retval = BAD_COND;
        goto cont;
    }

    if (name == null || name == "") {
        retval = BAD_NAME;
        goto cont;
    }

    if (value == 0) {
        retval = BAD_VALUE;
        goto cont;
    }

    if (!perms.allow(name)) {
        retval = PERM_DENY;
        goto cont;
    }

    else
        // do something on success

cont:
    // do something in any case

    return retval;
}


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

public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
{
    int retval = SUCCESS;

    if (!someCondition)
        retval = BAD_COND;

    else if (name == null || name == "")
        retval = BAD_NAME;

    else if (value == 0)
        retval = BAD_VALUE;

    else if (!perms.allow(name))
        retval = PERM_DENY;

    else
        // do something on success

    // do something in any case

    return retval;
}

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.

(defun f ()
  (prog1
    (block nil
      (cond ((not test1) (return 'TEST1-FAILED))
            ((not test2) (return 'TEST2-FAILED))
            ((not test3) (return 'TEST3-FAILED))
            ((not test4) (return 'TEST4-FAILED)))
      ; ...
      ; do something on success
      ; ...
      'SUCCESS)
    ; ...
    ; do something in any case
    ; ...
  ))

Autor: no goto (Gast)
Datum:

Bewertung
-2 lesenswert
nicht 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?"

Autor: Nur so eine Idee (Gast)
Datum:

Bewertung
-1 lesenswert
nicht 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.

Autor: Timm R. (Firma: privatfrickler.de) (treinisch)
Datum:

Bewertung
1 lesenswert
nicht 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:
    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
Autor: Tom (Gast)
Datum:

Bewertung
-1 lesenswert
nicht 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?

Autor: Achim H. (anymouse)
Datum:

Bewertung
1 lesenswert
nicht 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.

Autor: DPA (Gast)
Datum:

Bewertung
0 lesenswert
nicht 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.

Autor: Vincent H. (vinci)
Datum:

Bewertung
0 lesenswert
nicht 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.

Autor: Tom (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Danke für eure hilfreichen Antworten :-)

Autor: Oliver S. (oliverso)
Datum:

Bewertung
-2 lesenswert
nicht 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

Autor: mh (Gast)
Datum:

Bewertung
2 lesenswert
nicht 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:
Explanation
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
expects: i > 0

Das liest sich nicht sehr statisch.

Autor: Sven B. (scummos)
Datum:

Bewertung
1 lesenswert
nicht 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.

Autor: Oliver S. (oliverso)
Datum:

Bewertung
1 lesenswert
nicht lesenswert
Ups, ich hatte „concepts“ gelesen. Freud‘sche Fehlleistung.

Olivet

Antwort schreiben

Die Angabe einer E-Mail-Adresse ist freiwillig. Wenn Sie automatisch per E-Mail über Antworten auf Ihren Beitrag informiert werden möchten, melden Sie sich bitte an.

Wichtige Regeln - erst lesen, dann posten!

  • Groß- und Kleinschreibung verwenden
  • Längeren Sourcecode nicht im Text einfügen, sondern als Dateianhang

Formatierung (mehr Informationen...)

  • [c]C-Code[/c]
  • [avrasm]AVR-Assembler-Code[/avrasm]
  • [code]Code in anderen Sprachen, ASCII-Zeichnungen[/code]
  • [math]Formel in LaTeX-Syntax[/math]
  • [[Titel]] - Link zu Artikel
  • Verweis auf anderen Beitrag einfügen: Rechtsklick auf Beitragstitel,
    "Adresse kopieren", und in den Text einfügen




Bild automatisch verkleinern, falls nötig
Bitte das JPG-Format nur für Fotos und Scans verwenden!
Zeichnungen und Screenshots im PNG- oder
GIF-Format hochladen. Siehe Bildformate.

Mit dem Abschicken bestätigst du, die Nutzungsbedingungen anzuerkennen.