Methode zu lang - Toxizitäts Metriken - FixInsight

[creehawk]

Aha.

Danke für die Antworten Links, habe ich gelesen.

Zusammengefasst: Taten sind eher nicht notwendig. Was ich am meisten gelesen habe ist das Thema Übersichtlichkeit und aussagekräftige Benennung von Funktionen. Das die Teile dann ~ 80 Zeilen lang werden tut in unserem Fall der Sache keinen Abbruch. Zumal wir uns die Mühe machen - nicht zuletzt damit wir es auch noch nächstes Jahr verstehen - ausgiebig Kommentare zu verwenden. Auch die Namensgebung haben wir an die Lesbarkeit angepasst. Unser geprüftes Programm dient zur Erzeugung von Spielkarten Skripten für ein Computerspiel (Age of Empires), die angesprochene Function ist vom Namen her für jeden, des sich Programm ansieht und das Spiel kennt, völlig klar.

Wenn es also nur darum geht ob ich mal scrollen muss kann es mir also egal sein.

Ich hatte den Begriff Funktion bisher so aufgefasst: Bündeln einer öfter wiederkehrenden Aufgabe. Oder bündeln eines Abschnittes (bzw.mehrere Abschnitte) die man nach Bedarf einmal aufruft oder nicht. Beispiel: Winterlandschaft oder Sommerlandschaft.

Jedenfalls gehe ich davon aus das es Delphi wurscht ist ob die Funktion nun 20 oder 150 Zeilen lang ist. Oder liege ich falsch?

Toxizitäts Metriken lege ich jedenfalls unter 'Brauch` ich nicht' ab. In unserem Falle. Sollte ich mal ein Programm schreiben sollen für die Steuerung eines Raumschiffes kann ich mir das ja nochmal ansehen.

creehawk

[himitsu]

Jupp, Delphi ist es egal ... da kann am Ende auch der ganze Code nur in einer einzigen Funktion liegen. (z.B. bei einem Konsolenprogramm alles in der DPR nach dem BEGIN)

Es geht da wirklich nur um die Übersichtlichkeit/Wartbarkeit des Codes.
Im weiteren Sinne auch im die Testbarkeit. Ist eine große Methode nochmal unterteilt, dann kann man die Einzelteile auch getrennt testen.





Und notfals macht man sich eine {$REGION} in die Methode und faltet alles zu einer Zeile zusammen.

[OlafSt]

Ja, hier hat sich einer einen Dreck um sowas geschert... Eine DLL, dient dem Ausdruck von Angeboten, Gesamt-, Zwischen- und Endrechnungen, Gutschriften, Mahnungen, Sammelrechnungen, Stornierungen und so weiter. Eben alles, was an Ausdrucken in einem Geschäft so anfällt. Außerdem werden diese Dokumente in PDF's übertragen, in der Datenbank wird alles sauber verbucht, dann für SAP zusammengestellt und in die benutzerabhängigen Dokumentablagen kopiert.

Alles in einer (in Zahlen: 1), 11k Zeilen langen Methode.

Darin einen Fehler zu suchen oder was zu ändern entfärbt die Haare besser als jeder Wasserstoffperoxid.

[Der schöne Günther]

Rekordverdächtig.

Da lobt man sich doch Refactoring-Tools wie MMX das einen einfach Text markieren und mit einem Hotkey in eine neue Methode auslagern lässt

[OlafSt]

Habe ich schon versucht. Von den MMX-Tools habe ich erst kürzlich was mitbekommen und bin begeistert. Aber gegen sowas hier:

Delphi-Quellcode:

if (s_Art='Rechnung') or (s_Art='Endrechnung') or (s_Art='Gutschrift') then
begin
   //900 Zeilen Code
   if (s_Art = 'irgendwas') then
      ...
end;

if (s_Art='Angebot')  then
begin
   //300 Zeilen Code
end;

Da ist nix mit Refactoring und/oder Auslagern in extra Methoden. Das Ding ist ein Meisterwerk des unentwirrbaren Spaghetti-Codes.

[himitsu]

Delphi-Quellcode:

if MatchStr(s_Art, ['Rechnung', 'Endrechnung', 'Gutschrift']) then
   MethodeFürRechnungen(...);

if s_Art = 'Angebot' then
   MethodeFürAngebote(...);

[Uwe Raabe]

Zitat von himitsu:

Delphi-Quellcode:

if MatchStr(s_Art, ['Rechnung', 'Endrechnung', 'Gutschrift']) then
   MethodeFürRechnungen(...);

if s_Art = 'Angebot' then
   MethodeFürAngebote(...);

oder

Delphi-Quellcode:

if IsRechnungsArt(s_Art) then
   MethodeFürRechnungen(...);

if IsAngebotsArt(s_Art) then
   MethodeFürAngebote(...);

oder, wenn es sich gegenseitig ausschließt

Delphi-Quellcode:

if IsRechnungsArt(s_Art) then
   MethodeFürRechnungen(...)
else if IsAngebotsArt(s_Art) then
   MethodeFürAngebote(...);

oder

Delphi-Quellcode:

case GetArt(s_Art) of
  artRechnung: MethodeFürRechnungen(...);
  artAngebot: MethodeFürAngebote(...);
  ...
else
  raise EInvalidArtExcetion.Create('unbekannte Art: ' + s_Art);
end;

Auch der weiteste Weg beginnt mit einem ersten Schritt. (Konfuzius)

[sh17]

Zitat von OlafSt:

Habe ich schon versucht. Von den MMX-Tools habe ich erst kürzlich was mitbekommen und bin begeistert. Aber gegen sowas hier:

[Delphi]
if (s_Art='Rechnung') or (s_Art='Endrechnung') or (s_Art='Gutschrift') then
begin
//900 Zeilen Code
if (s_Art = 'irgendwas') then
...
end;
Da ist nix mit Refactoring und/oder Auslagern in extra Methoden. Das Ding ist ein Meisterwerk des unentwirrbaren Spaghetti-Codes.

Ablaufsteuerung durch casesensitive Strings - krass - nicht mal Konstanten verwendet, ich hoffe es gibt wenigstens ein Handbuch zur Funktion

[OlafSt]

Nope. Das ganze ist auch absolut hervorragend dokumentiert und kommentiert. Kommentare sind drin, jede Menge: Auskommentierter Code, weil geändert und Fehlerbehebungen mit Kürzel wer und wann.

Kommentare mit erklärender Funktion gibt es - wenn man meine hinzugefügten abzieht - genau gar keinen.

Wäre die Routine so aufgebaut, wie die Vorschreiber es sich vorstellen, hätte ich mich da auch mal dran gewagt. Ihr geht davon aus, das das ganze logisch nach Funktion gegliedert ist. Das würde das Refactoring tatsächlich vereinfachen. Aber so ist das Ding nicht gebaut.

Das Teil ist so strukturiert, wie ein Drucker arbeitet . Am Anfang des Blattes gibt es [Code]. Die Zeile darunter ist aber für Kunden "BCD" anders, also [Code mit Ausnahme]. Dann muß da ja n Firmenlogo kommen, also [Je nach s_Art [je nach Kunde [Code]]. Hab ich schon das Seitenende erreicht ? Ach ja, englische Texte brauchen wir ja auch noch... Und so weiter. Zusätzlich wird etwa ein dutzend SQL-Queries geöffnet, je nach s_Art und Kunde mit anderen Statements versehen; da werden Formulare geöffnet (weil die nicht wußten, wie man sonst Klassen erzeugt) und in versteckte Edit-Felder Werte eingetragen (keine Ahnung von Properties gehabt) und dann im OnChange-Handler jeweils s_Art- und Kunde-Abhängige Dinge gemacht. Wirklich benutzt wird das dann 5k Zeilen weiter unten...

Wie schon gesagt, Spaghetti-Code vom allerfeinsten. Das auseinander zu sortieren hat schon mal einer versucht und es nach 3 Monaten aufgegeben, weil die zahllosen Seiteneffekte einfach nicht mehr beherrschbar waren.

Letztlich geht es aber nicht um dieses "Meisterwerk der Software-Entwicklung". Man kann tatsächlich Code produzieren, mit dem man nur noch eines machen kann: Löschen, weil selbst so coole Tools wie MMX einfach nicht mehr gegenan kommen. Zum Glück ersetzen wir diesen Müll sukzessive durch wartbaren und besser dokumentierten (weil meinen ) Code.