spagetticode in methoden strukturieren & ordnen

[MB]

moin community,

ich habe in einem video gesehen das der Tutor spagetticode in einer methode produziert. Ist nicht weiter tragisch und schränk auch niocht die funktionalität ein, aber ür meine begriffe unschön. Mir viel auf, dass das doch schöner in weitere Methoden zu gliedern ist. Auch fürt die seperierung von Funktionalität in methoden. Es ist überschaubarer und man weis, was dieser codeabschnitt mit dem(n) parameter(n) macht. NetBeans meckert wenn man über 20 zeilen hinaus einem Methode schreibt.

Ist das sehr individuell oder ist das ein genereller Stil einer Codeconvention wenn man ziehmlich deterministisch arbeitet?

Tutor Code

class myClass {
 
  public myClass( quz ) : type {
    /* sehre viele Zeilen */
    return c;
  }
}

Mein Code

class myClass {
 
  public myClass( quz ) : type {
    var a = this.foo( quz );
    var b = this.bar( foo );
    var c = this.tok( bar );
    return c;
  }

  /* deterministische algorithmen */
  private foo( type ) : type { /* ... */ }
  
  private bar( type ) : type { /* ... */ }
  
  private tok( type ) : type { /* ... */ }
}

vlg MB

[Rolf b]

(Wasn das für ne Programmiersprache? Sieht aus wie eine Klasse mit Konstruktor, aber ein Konstruktor darf nichts zurückgeben. Zumindest nicht in Java oder PHP...)

Zum einen kannst Du der Netzbohne sagen, ab wievielen Zeilen sie meckern soll.

Zum anderen finde ich ein Zeilenlimit wenig geeignet, um eine Methode als "schlecht strukturiert" zu brandmarken. Manchmal muss man eben stumpf was runterprogrammieren, z.B. 17 Parameter an ein SQL Statement binden, und dann wird das Ding nun mal was länger. Da fehlt der Netzbohne etwas, das ich am Resharper schätze: Eine Direktive namens "Halt HIER mal die Klappe".

Das Kriterium für den Schnitt von Methoden heißt: Do One Thing. Eine Methode soll eine Aufgabe lösen. In „Clean Code“ erzählt Robert C. Martin die folgende Anekdote. Ich habe das Buch in meinem Kindle, und da geht kein Cut+Paste als Text, daher snippe ich es als Bild heraus...

Ich kann das Buch nur empfehlen. Allerdings muss man auch immer den Trade-Off im Auge behalten: Funktionsaufrufe kosten Zeit. Ein guter Compiler kann durch Inlining viel optimieren, aber nicht jede Sprache hat so etwas.

Manchmal ist es auch sinnvoll, eine Methode in eine eigene Klasse auszulagern. Wenn man eine Methode programmiert, die immer länger wird, und man ggf. noch Helperfunktionen aufruft, die 8 Parameter brauchen - dann ist das ein untrügliches Zeichen für einen Worker. Den erzeugt man und ruft darin eine Methode auf, die den Job erledigt. Statt wilden Parameterübergaben zu Helpern kann der Worker Instanzvariablen verwenden. Und das schöne ist: Du kannst ihn autark Unit-Testen. Du kannst ihn vielleicht auch in der nutzenden Klasse durch einen Mock ersetzen und damit die Tests dieser Klasse vereinfachen.

Rolf

[MB]

Danke Dir!

PS: war nur Pseudocode

[encoder]

Ich habe schon Code gesehen der mehrere tausend Zeilen pro Klasse hatte, da sollte man mehrere Klassen draus machen.
Ich habe Methoden gesehen die etliche Seiten zu scrollen hatten, die sollte man dringend aufteilen.

Andrerseits wenn eine Methode über drei Seiten geht, aber trotzdem gut strukturiert und nachvollziehbar ist, kann das in bestimmten Fällen auch sinnvoll sein. Alles auseinander zu reißen, viele Parameter an einzelne Methoden zu übergeben und beim lesen dauernd hin und her springen zu müssen ist ja auch nicht per Definition schön.
Der von Rolf erwähnte Code mit nur max. 4 Zeilen pro Methode mag schön aussehen, aber ist er auch nachvollziehbar? Es gibt Dinge die brauchen nunmal Code, was versteht man davon noch wenn man nach bereits vier Zeilen schon wieder woanders hin springen muss? Was passiert zum Beispiel mit einem case Statement mit vielen Fällen?
Also wo will man dann eine Zahl als Grenze fürs meckern festlegen?

Solche fest gezurrten Regeln verfehlen den Sinn der Sache. Vor allem wenn dann noch absolute Zahlen im Spiel sind. Sich gedankenlos daran zu halten macht nichts besser. Da geht es nicht anders als dass es eben länger wird.

Meldungen zum Programmierstil nutze ich höchstens als Hinweis, manchmal sind sie ja wirklich hilfreich. Manchmal aber auch nicht, dann nehme ich mir die Freiheit sie zu ignorieren oder auszuschalten.

[Rolf b]

Das switch-statement mit den vielen cases ist ein typischer Fall für Do One Thing Probleme. Wenn in den cases jeweils steht, was zu passieren hat, ist es meistens nicht mehr clean. Dann bietet sich eine Methode für die Fallunterscheidung an und jeder Fall ist eine eigene Methode. Ein Switch ist aber auch oft ein Indiz für verpasste Polymorphie, und wenn sich einer einschleicht, sollte erstmal die Warnglocke läuten.

Es sei denn, jeder Fall tut nichts weiter als einem Wert A einen Wert B zuzuordnen. DANN sollte die Methode aber auch nichts weiter als diesen switch enthalten, danach zugeklappt werden und weggesperrt 😉. Oder vielleicht durch eine Lookup-Hashtabelle ersetzt werden.

Wenn Du viele Parameter übergeben musst, kann das für Methodenkohärenz sprechen. D.h. diese Methoden gehören zusammen in eine gemeinsame Klasse ausgelagert. Das ist der von mir erwähnte Worker.

Für ältere Praktikanten der Programmierkunst sind solche Überlegungen fremd bis schmerzlich. In den 80ern sah Code einfach anders aus. Inlining war eine Disziplin, die Compiler nicht unbedingt beherrschten, und man ersetzte es durch Makros. Funktionsnamen waren kurz und knackig (weil Editoren mit intelligenter Code-Vervollständigung fehlten).

Methodennamen bei Martin sind halbe Romane, du liest den Methodennamen und weißt schon, was die Funktion tut. Nicht WIE sie es tut. Das ist ja auch wurscht, solange Du weißt, WAS es ist. Dann springst Du nämlich nicht hin und her. Du siehst einen Aufruf

   var endKapital = berechneEndKapitalNachLaufzeit(grundKapital, zinssatz, laufzeitInJahren);

und musst eigentlich in die berechne-Methode nicht hineinschauen. Es sei denn, du hast Grund zur Annahme, dieser Methode zu misstrauen. Aber DANN guckst Du in die Unit-Tests für diese Methode und schaust, was da getestet wird. Ist der Fall, dem Du misstraust, nicht dabei - okay, schreib einen Test dafür. Lass die Tests laufen. Geht dein neuer Test durch, ist dein Misstrauen unbegründet und du hast eine Lücke im Test geschlossen.

Natürlich gehört zu Clean Code noch viel mehr. Martin empfiehlt, möglichst wenige Parameter zu verwenden, und Seiteneffekte sind des Teufels.

Martin schreibt auch, dass er bei einem neuen System nicht mit Clean Code beginnt. Seine Methoden sind zu lang, zu komplex, die Namen blöd gewählt. Er bringt es erstmal zum Laufen und schreibt Tests, die diese Methoden absichern. Und dann beginnt er, graduell auszulagern, zu zerlegen, zu refaktorieren. Solange die Tests weiter durchgehen, macht er alles richtig. Wenn nicht - na gut. Dann muss man gucken ob der Test ungenau oder die Refaktorierung blöd war.

Clean Code direkt von Anfang an schreiben? Das kann niemand, meint Martin. Erstmal schreiben. Unittests schreiben. VIELE. Und dann refaktorieren, bis es gut ist.

Rolf

[encoder]

Du hast ja Recht mit dem was du sagst. Ich will nur feststellen dass man keinen clean Code erreicht indem man möglichst viele Regelungen mit festen Zahlen und absoluten Angaben in den Raum wirft.
Dann zählt man die Zeilen einer Methode und beurteilt anhand größer/kleiner x. Man teilt 22 Zeilen in 2x11 auf oder zerfleddert Code auf andere Art und denkt das macht alles besser, nur weil es so vorgeschrieben ist. Mit Fachkenntnis hat sowas nichts mehr zu tun.

Es gibt übrigens schon auch auch Fälle in denen man viele case Statements hat und das die beste Wahl ist. Wenns dann 50 case Einträge sind, so what? Das lagert man aus, dann ist es in anderem Code nicht mehr im Weg. Wenn diese Funktion dann 50*x Zeilen lang ist, so what? Eine Hashtable musst du doch auch irgendwo füllen, sind auch 50 Zeilen Code.

[Rolf b]

Hallo encoder,

ich widerspreche Dir ja gar nicht. Es ist immer eine Situationsbetrachtung, und absolute Zahlen für Methodenlängen sind genau das, was Du meinst: Für Leute ohne Fachkenntnis. Wie z.B. eine IDE 😉. Wenn Du für Dich sagen kannst: Diese Methode erfüllt den "Do One Thing, And Do It Right" Anspruch, dann können Dir Zahlen egal sein.

Dein switch mit 50 case-Labels, die alle eigenen Code beinhalten, hat aber schon einen leichten Duft nach Bad Design. „Smell“ heißt, dass da was im Argen sein KANN. Es kann genauso gut sein, dass es Zwänge für dieses Design gibt. Und dann ist es gut so. Robert Martin würde wahrscheinlich durchdrehen, aber so ist das nun mal mit den Zeloten...

Rolf