Hallo Community,
Es ist nur eine Kleinigkeit, aber trotzdem interessiert mich eure Meinung.
Angenommen in einer Methode gibt es eine Bedingung, welche über den Rückgabewert entscheidet - kommt ja häufiger mal vor. 😉 Nun sehe ich drei Möglichkeiten dies zu lösen und ich frage mich, welche (und warum) am sinnvollsten ist.
Möglichkeit 1:
string Foo(bool b) {
string str = "bar";
if(b) {
str = "baz";
}
return str;
}
Möglichkeit 2:
string Foo(bool b) {
string str = string.Empty;
if(b) {
str = "baz";
}
else {
str = "bar";
}
return str;
}
Möglichkeit 3:
string Foo(bool b) {
if(b) {
return "baz";
}
else {
return "bar";
}
}
Meine Meinung dazu:
M3 kommt vielleicht noch in Frage wenn die Methode nicht viel komplexer ist, als in diesem Beispiel, sobald aber mehr Code vorhanden ist, wird es echt unübersichtlich. Grundsätzlich bin ich auch gegen mehrere Return-Statements innerhalb von einer Methode.
Nun schwanke ich zwischen M1 und M2. Früher habe ich eigentlich immer M1 verwendet, da sie weniger Zeilen braucht. Jetzt tendiere ich eher zu M2, weil, wie ich finde, dort sofort ersichtlich ist, was passiert.
Klar ist, dass wenn anstelle der simplen Zuweisung str = "bar" in M1 eine teure Operation stattfinden würde, ich natürlich auch M2 verwende.
Wie handhabt ihr das?
Gruß,
rastalt
Hallo rastalt,
das ist eine Glaubensfrage, die wir schon mehrfach besprochen haben.
Es gibt Leute, die bestehen darauf, dass eine Methode nur ein return haben darf. Und es gibt Leute wie ich, die sagen, dass man bei jedem Codepfad, an dessen Ende der Rückgabewert feststeht, unbedingt auch ein return verwenden sollte. Dadurch kann man beim Lesen des Code den Zweig gedanklich abhaken und spart oft gleichzeitig Einrückungstiefe.
herbivore
Wobei Möglichkeit 2 unsinnig ist, da das letzte Return definitiv nie erreicht würde. Das müsste dir eigentlich sogar der Compiler sagen.
Daher würde ich die Methode so aussehen lassen:
string Foo(bool b) {
return (b ? "baz" : "bar");
}
zusätzlich habe ich festgestellt, das die "nur ein return" philosophie manchmal zu methoden führt, wo ein flag gesetzt wird und dieses sehr oft geprüft wird, damit man endlich zum return kommt.
z.b. :
public string foo()
{
string result;
bool meinfalg = true;
if(x)
{
result = "XXX";
meinflag = false;
}
if(meinflag && x)
{
result = "XXX";
meinflag = false;
}
if(meinflag && x)
{
result = "XXX";
meinflag = false;
}
if(meinflag && x)
{
result = "XXX";
meinflag = false;
}
if(meinflag && x)
{
result = "XXX";
meinflag = false;
}
return result;
}
das ist meiner meinung nach hässlich und unpraktisch.
Wobei Möglichkeit 2 unsinnig ist, da das letzte Return definitiv nie erreicht würde. Das müsste dir eigentlich sogar der Compiler sagen.
Daher würde ich die Methode so aussehen lassen:
string Foo(bool b) { return (b ? "baz" : "bar"); }
Wenn man diese Schreibweise nach einer Weile noch versteht, dann ok.
@rastalt
ich zitiere einfach mal aus clean-code-developer.de
Eine einfache, klare, leicht verständliche Lösung sollte daher immer bevorzugt werden
Soll heißen, das Du oder jeder andere Entwickler Deinen Code lesen und verstehen sollte.
MfG lbm1305
Debugger sind was für Memmen! Echte Männer kompilieren direkt auf die Master CD …
Daher würde ich die Methode so aussehen lassen:
string Foo(bool b) { return (b ? "baz" : "bar"); }
Würde bei mir genauso aussehen.
das ist eine Glaubensfrage, die wir schon mehrfach besprochen haben.
Es geht mir nicht darum, ob mehrere Returns sinnvoll sind oder nicht und ich bin mir auch bewusst, dass das schon mehrmals besprochen wurde. Es geht mir eher darum ob man eher M1 oder M2 oder ganz was anderes benutzen sollte.
Wobei Möglichkeit 2 unsinnig ist, da das letzte Return definitiv nie erreicht würde. Das müsste dir eigentlich sogar der Compiler sagen.
Bei mir kompiliert das einwandfrei und ich sehe da auch logisch keinen Fehler drin.
Daher würde ich die Methode so aussehen lassen:
string Foo(bool b) { return (b ? "baz" : "bar"); }
Da hast du recht, die Möglichkeit habe ich noch nicht in Betracht gezogen. Allerdings kann das auch sehr schnell unübersichtlich werden, wenn man z.B. je nach nach Wert von b das eine oder andere SQL-Statement zurückgeben will. Außerdem kann es ja vorkommen, dass man im Fall von b = true erst noch weitere Operationen ausführen will. In meinem Beispiel wäre deine Lösung zu bevorzugen, aber ich habe ja auch bewusst ein simples gewählt.
In dem Fall würde ich wohl eine - ich nenn's mal - "Junction" Methode schreiben:
SomeRetVal Junction(Whatever b)
{
SomeRetVal ret = null;
switch(b)
{
case Whatever.FooA:
{
ret = DoSomeThingA();
break;
}
case Whatever.FooB:
{
ret = DoSomeThingB();
break;
}
default:
{
ret = DoSomeThingDifferent();
break;
}
}
return ret;
}
Finde ich persönlich am schönsten (wie gesagt, Glaubensfrage). Die ganzen DoSomeThing... Methoden würde ich dann in eine #region packen.
Hallo rastalt,
nach dem von mir gesagten M4:
string Foo(bool b)
{
if(b) {
// < hier mehr Code >
return "baz";
}
// < hier mehr Code >
return "bar";
}
Jeden Pfad mit return abschließen, wenn der Rückgabewert feststeht.
herbivore
Gut, nach Herbivores Vorgehensweise kann man die "Junction" dann natürlich auch so bauen, was die Lesbarkeit tatsächlich noch erhöht:
SomeRetVal Junction(Whatever b)
{
switch(b)
{
case Whatever.FooA:
{
return DoSomeThingA();
}
case Whatever.FooB:
{
return DoSomeThingB();
}
default:
{
return DoSomeThingDifferent();
}
}
return null; // wird auch wieder niemals erreicht. Sollte zumindest eine Compiler-Warning geben
}
Hallo,
da der Fokus auf der Lesbarkeit liegen sollte, erscheint es mir sinnvoll, die tatsächliche Code-Variante vom konkreten Fall abhängig zu machen, z. B. also zu entscheiden, ob ich eine if-Variante oder den ?-Operator benutze.
Insofern bin ich für herbivores "M4" oder bei einfachen Rückgaben die ?-Variante.
Bei komplexeren Methoden muss man allerdings aufpassen, dass man wirklich an jedem möglichen Ausstiegspunkt ein return baut und vorher auch aufräumt, falls nötig.
Ich erinnere mich an meine zeit als C++ Entwickler, wo dann Speicherlecks entstanden, weil von meinen werten Kollegen an etlichen Stellen in riesigen unübersichtlichen Methoden _return_s, aber nicht überall _delete_s waren. Da blieben dann am Anfang der Methoden mit new angelegte Objekte einfach im Speicher liegen.
Okay, bei C# ist das nicht mehr so tragisch wegen des GC. Aber auch hier kann man damit Speicherlecks bauen, wenn man wild IDisposable Objekte anlegt (beim Arbeiten mit GDI+ zum Beispiel).
Hallo 7.e.Q,
da man typischerweise im finally aufräumt, gibt es mit mehreren returns eigentlich keine Probleme mehr.
herbivore
Zitat von: herbivore
Hallo rastalt,nach dem von mir gesagten M4:
string Foo(bool b) { if(b) { // < hier mehr Code > return "baz"; } // < hier mehr Code > return "bar"; }
Jeden Pfad mit return abschließen, wenn der Rückgabewert feststeht.
herbivore
So mach ichs auch immer. Ein else ist da unnötig und es ist vor allem schön zu lesen, da es einfach nur das Nötigste enthält.
Ich finde, dass die Möglichkeit mit dem einzigen Return am Ende der Methode nur in Frage kommt/kommen sollte, wenn nach dem Setzen des Rückgabewerts anschließend noch andere Operationen durchgeführt werden. Ansonsten sehe ich keinen Sinn darin.
> Codejunky <
Äh, "finally"? Das gibt's doch nur in Verbindung mit 'nem Try-Catch-Block oder? Oder hab ich was verpasst?
Hallo 7.e.Q,
nur in Verbindung mit einem try. Ein catch ist nicht erforderlich.
finally dient dazu, sicher aufzuräumen, egal warum und wo die Methode verlassen wird. Das schließt das Verlassen im Fehlerfall ein, ist aber nicht auf Fehlerfälle begrenzt.
herbivore
Wunderbar, die Diskussion ist ja - nach ein paar Startschwierigkeiten - doch in die richtige Richtung gegangen. 🙂
In Zukunft werde ich dann wohl die Variante mit dem bedingten Operator verwenden. Wenn es dann doch mal ein bisschen komplexer wird werde ich auf M4 zurückgreifen und ggf. auch auf ein Try-Finally-Block.
Sorry für OT, aber Verständnisfrage: wenn ich aus einem Try-Block mit return aussteige, wird der Finally-Block also noch durchlaufen, bevor die Methode letztendlich zurückkehrt?
ja
! immer !
egal wie Du den try-Block verlässt, ohne return, mit return oder mit throw new Exception()
mbg
Rossegger Robert
mehr fragen mehr wissen
Montag morgen ist die beste Zeit um eine erfolgreiche Woche zu beginnen
Gut zu wissen
Deswegen sollte man ja bei lokalen GDI-Objekten auch "using(new X(...))" verwenden, da dieser intern als "try...finally" implementiert ist. (gilt natürlich generell für alle IDisposable-Klassen)