Laden...

Komplexe lange Methode mit vielen If's besser strukturieren

Erstellt von Unfug vor 6 Jahren Letzter Beitrag vor 6 Jahren 2.552 Views
U
Unfug Themenstarter:in
133 Beiträge seit 2006
vor 6 Jahren
Komplexe lange Methode mit vielen If's besser strukturieren

Hallo mycsharp,

ich habe heute eine Frage in Richtung Refactoring.
Und zwar folgende Situation:

Ich habe eine Methode die mir ein komplexes Objekt zurückliefert. Hierfür holt die Methode Daten aus der Datenbank und macht eine Menge Fallunterscheidungen (If Anweisungen).

Die IF Anweisungen sind zum teil stark verschachtelt


If(..)
{
    If(...)
   {
        If(...)
         {
            obj.ListA(new A());
        }
    }
}

Bei einem True in der letzten IF Anweisung füge ich dem Objekt z.B. etwas in einer Objekt.List hinzu.
Soweit nichts besonderes. Meine Methode ist allerdings 2000 Code Zeilen lang. Ich habe Sie zunächst runterprogrammiert. Die IF Anweisungen habe ich in unterschiedliche Regionen unterteilt, da sie im Prinzip jeweils eigenständig sind. In der Methode selbst nutze ich auch noch 4 lokale Funktionen.

Jetzt wollte ich das Ganze ein wenig sauberer machen, um es auch wartbarer zu machen. Mein Problem ist: Was ist die Beste Denk- und Vorgehensweise eine derart komplexe Methode richtig zu refactoren.

Mein erster Ansatz war, alle Regionen auslagern in eine eigene private Methode.

Somit hätte ich in etwa sowas


RefactoredMethod()
{
   var obj = new komplex();
   AddToObjektRegionA(obj);
   AddToObjektRegionB(obj);
   AddToObjektRegionC(obj);
   AddToObjektRegionD(obj);
   AddToObjektRegionE(obj);
....
}

Ich komme hierbei auf 28 Methoden, die ausgelagert sind.
Jetzt habe ich die Klasse mit privaten Methoden zugemüllt, die nur von dieser eine RefactoredMethod genutzt werden.

Normalerweise halte ich mich an das Prinzip: Methode = 1 Aufgabe und nicht zu lang. Was allerdings hier wirklich schwer ist.
Das komplexeObjekt wird später Teil eines ViewModels sein und muß als Ganzes abgerufen werden.
Ich kann also nicht die einzelnen Methoden public machen und diese nutzen.

Wie geht ihr mit so langen Methoden um, die euch ein Objekt aufbauen? Einfach so lassen? Mit privaten Methoden?
Übrigens.: Die IF Anweisungen sind alle schon optimiert. Hier kann ich nicht viel machen. Es handelt sich hierbei um individuelle Abfragen. Die eine verwendet das aktuelle Datum, die andere noch eine andere Quelle, um Daten zu vergleichen, die andere ein anderen Wert aus der Datenbank und und und.

Danke für die Ratschläge.

F
10.010 Beiträge seit 2004
vor 6 Jahren

Jetzt habe ich die Klasse mit privaten Methoden zugemüllt, die nur von dieser eine RefactoredMethod genutzt werden.

Komplett falsche Denkweise.
Wenn du es so siehst sind alle Privaten Funktionen vollmüllen, da sie ja nur intern benutzt werden.

Übrigens.: Die IF Anweisungen sind alle schon optimiert. Hier kann ich nicht viel machen. Es handelt sich hierbei um individuelle Abfragen. Die eine verwendet das aktuelle Datum, die andere noch eine andere Quelle, um Daten zu vergleichen, die andere ein anderen Wert aus der Datenbank und und und.

Hier liegt doch schon die Antwort.
Jede Funktion wird anhand ihrer Aufgabe benannt.

Aber ist es wirklich nötig das alles an einer Stelle und auch komplett durchzuführen?

W
955 Beiträge seit 2010
vor 6 Jahren

Du könntest mal schauen ob ein Entwurfsmuster dich inspiriert, Builder oder Chain of Responsibiklity beispielsweise.

U
Unfug Themenstarter:in
133 Beiträge seit 2006
vor 6 Jahren

Hallo FZelle,

ja du hast schon recht mit der Aussage bzgl. privater Methode. Ich hatte das Wort "vollmüllen" nur benutzt, weil die Methode ausschließlich von dieser einen Methode genutzt wird und dabei in der Regel nur einmal durch die RefactoredMethode benutzt wird. Mir kam das irgendwie nicht sauber vor.

Hier liegt doch schon die Antwort.
Jede Funktion wird anhand ihrer Aufgabe benannt.

Aber ist es wirklich nötig das alles an einer Stelle und auch komplett durchzuführen?

Die Methoden habe ich auch alles soweit korrekt benannt.

Leider muss alles durchgeführt werden. Man kann sich das ganze als eine Art Business Objekt vorstellen, dass mit unterschiedlichen Werten gefüllt wird. Jetzt wird es quasi durch die 28 unterschiedlichen Privaten Methoden gefüllt und an die weiter oben liegende Logik und UI weitergeben.
Es müssen auch immer alle 28 Methoden durchlaufen. Jede einzelne Methode ist wichtig.

Ich bin mir nur nicht sicher, ob man es so wirklich machen sollte oder es nicht eine schönere Variante gibt, welche ich derzeit nicht sehe.

@Witte: Dein Beitrag jetzt gerade gesehen. Danke ich guck mir die mal an.

C
2.121 Beiträge seit 2010
vor 6 Jahren

die if könnte man in eine eigene Methode auslagern, die man immer nach einem if verlässt wenn man nicht weitermachen will. Dann hast du auf der ersten Ebene viele

if (...)
  return;
if (...)
  return;

hier passiert etwas

das macht es schon viel übersichtlicher als wenn du viele Einrückungsebenen hast.
Den Rest auslagern halte ich auch keineswegs für "müllen", denn irgendwo muss die Arbeit nunmal gemacht werden. Wenn du das in passende Funktionsblöcke auslagerst wird es verständlicher.
Du könntest auch prüfen ob du verschiedene Klassen draus machen kannst, dann kapselst du noch besser.

T
461 Beiträge seit 2013
vor 6 Jahren

Hallo,

es stellt sich auch die Frage für BusinessObjects (wenn es sowas ähnliches ist), ob die Möglichkeit besteht, daß diese eine Methode mal überschrieben werden muß/sollte?

Wenn dem so ist, dann ist das Aufteilen in mehrerer Methoden umso besser, dann kann man individuell einzelne Bedingungen überschreiben, anstatt den gesamten Block..

Ich habe den Titel mal angepasst, so dass Suchende auch etwas damit anfangen können. EDIT: Ich sollte beim Wort "Shift" im Titel das "f" nicht vergessen... 😄

4.931 Beiträge seit 2008
vor 6 Jahren

Anstatt so etwas von Hand auszuprogrammieren, würde ich ja eher eine "Business Rules Engine" oder einen Code-Generator empfehlen.

U
Unfug Themenstarter:in
133 Beiträge seit 2006
vor 6 Jahren

Danke nochmal an alle.
Ich habe jetzt auch nochmal alles bisschen refactored und bin bei den Methoden geblieben. Inzwischen hat sich das auch bewährt. Einige Methoden hatten Bugs und so habe ich diese sofort finden können.

Das mit einer eigenen Engine hatte ich auch schon überlegt. Ggfs. sogar soweit, dass ich das auslagere in einer Scriptsprache.

Danke

49.485 Beiträge seit 2005
vor 6 Jahren

Hallo Unfug,

so schön die Objektorientierung in dem allermeisten Fällen ist, in bestimmten Situationen kommt sie einfach an ihre Grenzen.

Das scheint hier so ein Fall zu sein, in dem es nur begrenzt Verbesserungsmöglichkeiten gibt.

Mir fällt es zwar auch schwer, wenn ich in eine solche Situation kommt, es auszuhalten, dass es nicht viel besser geht, aber im Grunde bleibt einem kaum was anderes übrig.

Insbesondere bleibt am Ende manchmal die Erkenntnis, dass keine Umstrukturierung die Code-Menge und die prinzipielle Code-Komplexität nennenswert ändert und schon gar nicht reduziert. Jede Umstrukturierung es also letztlich niemanden einfacher macht, den Code zu verstehen.

Natürlich kann eine Aufteilung in einzelne Methoden bestehende Abhängigkeiten aufzeigen und begrenzen, etwa wenn eine Variable in der großen Methode an vielen verschiedenen Stellen verwendet wird/wurde. Wenn diese Abhängigkeiten aber unvermeidbar sind, weil der Variablenwert eben wirklich an vielen verschiedenen Stellen benötigt wird, dann kann eine Aufteilung zu langen Parameterlisten der einzelnen Methoden führen, was es nicht wirklich besser macht oder im schlimmsten Fall sogar schlechter.

Es kann also in solchen Ausnahmefällen - und damit meine ich wirklich nur in solchen wenigen(!), gut durchdachten Ausnahmefällen durchaus Sinn machen, es bei der einen großen Methode zu belassen, und die Energie, die man für das Refactoring aufzuwenden bereit ist/wäre, lieber darein zu investieren, die Methode ausführlich und gut zu kommentieren/dokumentieren.

Zumal eine große Methode die (Meta-)Message klar(er) transportiert, dass wirklich alles nötig ist und alles immer durchlaufen werden muss, und es sich gerade nicht um einzelne Bauteile handelt, die man nach Belieben auswählen kann.

herbivore

3.003 Beiträge seit 2006
vor 6 Jahren

Lässt sich anhand der Bruchstücke ganz schwer einschätzen, aber eventuell hilft dir so ein Konstrukt hier:


//init
var conditionalSelector = new ConditionalSelector<Action<MyClass>, Func<int,int,int,bool>>();
conditionalSelector.Register(obj => obj.ListA(new A()).For((a, b, c) => a > b && c % a > 1);

//später irgendwo:
foreach(var action in conditionalSelector.Where(a, b, c)
    action(obj)

Der ConditionalSelector ist eine Klasse, die beliebige Paare von Delegaten speichert (den auszuführenden Code und eine Bedingung, unter der er auszuführen ist. So etwas kann helfen, wenn man sehr viele strukturell ähnliche Aktionen zu je einer oder wenigen Zeilen Code für verschiedene Bedingungen ausführen will. Ist natürlich bei der Implementierung des ConditionalSelectors erst einmal etwas mehr Aufwand (dafür mMn wesentlich aufgeräumterer Code).

Insgesamt ist das auch nicht weniger Code dann, aber ich teile herbivores Auffassung nicht, dass nur reduzierte Codemenge es leichter macht, Code zu verstehen.

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

49.485 Beiträge seit 2005
vor 6 Jahren

Hallo LaTino,

vorne weg: ich bin grundsätzlich ein Freund von solchem Infrastruktur-Code, wie du ihr hier vorschlägst und schätze um seine Vorteile. Insbesondere die Vermeidung von Redundanz und die Möglichkeit eines zentralen Eingriffs, z.B. beim Loggen und Debuggen. Ich habe in meinem Beitrag nur gesagt, dass es einige, ganz wenige Fälle gibt, in denen sich kaum etwas verbessern und sich insbesondere die Code-Menge nicht nennenswert reduzieren lässt.

Natürlich kommt es nicht nur auf die Code-Menge an, wenn es um Lesbarkeit und Verständlichkeit geht. In den allermeisten Fällen verbessert eine Strukturierung die Lesbarkeit.

Im konkreten Fall reden wir laut dem Thread-Ersteller über eine sehr umfangreiche Funktionalität, die eben keine erkennbare Struktur hat und die letztlich einfach vollständig ausgeführt werden muss. Klar gibt in dem Code ifs, aber auf Ebene der Abschnitte, die in den einzelnen Methoden ausgelagert wurden, muss jeder dieser Abschnitte durchlaufen werden. Und auch wenn man vielleicht Abschnitte theoretisch vertauschen könnte, entscheidet man sich hier wohl für eine sinnvolle Reihenfolge und durchläuft die Abschnitte sequentiell.

Das ist ein sehr einfach zu verstehendes Konstrukt. Insbesondere, wenn man - wie vorgeschlagen - die Abschnitte durch Kommentare nicht nur erklärt, sondern auch kennzeichnet und damit voneinander abgrenzt. Trotzdem bleibt die (Meta-)Message, dass alle Abschnitte durchlaufen werden müssen im Code zu erkennen.

In einem solchen - wie gesagt - seltenen Fall kann es tatsächlich sein, dass alle Änderungen - egal wie gut sie gemeint sind - die Lesbarkeit verschlechtern, weil eben die Botschaft, "Ja, es ist viel Code, aber es hilft eben nichts, alles und muss ausgeführt werden", verloren geht. Und weil dieser an sich einfache Aufbau schlimmstenfalls durch eine aufgezwungene Struktur zerfasert und/oder verschleiert wird.

Ich hatte schon Fälle, in denen ich mich vor lauter Struktur gefragt habe, was denn eigentlich letztlich passiert. Zerfaserte Strukturen können - und mehr will ich gar nicht sagen - die Lesbarkeit und Verständlichkeit auch erschweren. Nicht umsonst gibt es dass KISS-Prinzip.

Natürlich ist KISS kein Freibrief einfach immer allen Code einfach sequentiell hintereinander zu schreiben. Und normalerweise wird sich es fast von alleine anbieten, Code in einzelne kleine Methoden auszulagern und es werden sich sinnvolle Namen für diese Methoden geradezu aufdrängen. Dann sollte man das natürlich auch entsprechend strukturieren.

Doch wenn sich nach gründlicher Prüfung einfach keine redundanten Stellen finden lassen und sich auch keine halbwegs passenden Namen für einzelne Methoden anbieten, dann kann das lesbarste durchaus auch mal ein größere Methode sein. Lang bedeutet in einigen bestimmten Fällen nicht automatisch komplex oder schwierig zu verstehen.

Möglicherweise würde man, wenn man sich den konkreten Fall im Detail anschaut, doch noch Redundanzen und doch noch sprechende Methodennamen (die natürlich für eine Aufteilung sprächen) finden, denn unbefriedigende Fälle, die sich allen Verbesserungsversuchen widersetzen, sind wirklich selten.

Doch es gibt eben auch ein paar wenige Fälle, wo mehr Struktur die Sache nicht besser macht, manchmal sogar im Gegenteil.

Es geht also gerade nicht um ein Pauschalurteil, dass nur reduzierte Codemenge es leichter macht. Was jeweils für ein Fall vorliegt, muss natürlich jeder einzelne für sich entscheiden.

herbivore