Laden...

Wie Methode strukturieren, wenn der Rückgabewert von einer Abfrage abhängt?

Erstellt von rastalt vor 14 Jahren Letzter Beitrag vor 14 Jahren 1.572 Views
R
rastalt Themenstarter:in
234 Beiträge seit 2007
vor 14 Jahren
Wie Methode strukturieren, wenn der Rückgabewert von einer Abfrage abhängt?

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

49.485 Beiträge seit 2005
vor 14 Jahren

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

925 Beiträge seit 2004
vor 14 Jahren

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");
}

Gelöschter Account
vor 14 Jahren

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.

L
45 Beiträge seit 2008
vor 14 Jahren

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 …

T
574 Beiträge seit 2008
vor 14 Jahren

Daher würde ich die Methode so aussehen lassen:

  
string Foo(bool b) {  
    return (b ? "baz" : "bar");  
}  
  

Würde bei mir genauso aussehen.

R
rastalt Themenstarter:in
234 Beiträge seit 2007
vor 14 Jahren

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.

925 Beiträge seit 2004
vor 14 Jahren

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.

49.485 Beiträge seit 2005
vor 14 Jahren

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

925 Beiträge seit 2004
vor 14 Jahren

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
}

U
1.688 Beiträge seit 2007
vor 14 Jahren

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.

925 Beiträge seit 2004
vor 14 Jahren

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).

49.485 Beiträge seit 2005
vor 14 Jahren

Hallo 7.e.Q,

da man typischerweise im finally aufräumt, gibt es mit mehreren returns eigentlich keine Probleme mehr.

herbivore

1.665 Beiträge seit 2006
vor 14 Jahren

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.

925 Beiträge seit 2004
vor 14 Jahren

Äh, "finally"? Das gibt's doch nur in Verbindung mit 'nem Try-Catch-Block oder? Oder hab ich was verpasst?

49.485 Beiträge seit 2005
vor 14 Jahren

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

R
rastalt Themenstarter:in
234 Beiträge seit 2007
vor 14 Jahren

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.

925 Beiträge seit 2004
vor 14 Jahren

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?

S
443 Beiträge seit 2008
vor 14 Jahren

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

925 Beiträge seit 2004
vor 14 Jahren

Gut zu wissen

4.931 Beiträge seit 2008
vor 14 Jahren

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)