Spagetticode verstehen?

1. klar machen, was genau mit "umschreiben" gefordert ist.
2. den aktuellen code verstehen. was macht er, warum macht er es und wie ist die aufgabenstellung einzuordnen
3. abschätzen, ob die gewünschte änderung sich in den aktuellen code einfügen lässt.
4. hoffen, dass das klappt
5. code ändern oder refaktorieren.

refaktorieren ist grundsätzlich gut. sobald ein stück code an einer stelle angelangt ist, an der weitere anpassungen nur noch durch noch mehr spagetti code, copy & paste oder fiese hacks zu erlangen sind, ist eigentlich refaktorierung angesagt.

aber jetzt kommt das problem. wenn code besonders "spagettihaft" ist, kann refaktorierung zwei folgen haben:
- der neue code leistet nicht exakt dasselbe, wie der alte
- es dauert ewig

nicht umsonst ist refaktorierung die büchse der pandora einmal angefangen, kommt man vom hundertsten ins tausendste. besonders wenn code son gefrickel ist, wie es bei dir offensichtlich der fall ist, und wild mit anderem code vermischt ist.

manchmal ist es ein guter ansatz, ein neues interface zu definieren und hinter dieser fassade den alten code 1:1 zu verstecken. dadurch kann es leichter werden, neue änderungen anzubringen, da diese gegen das neue interface arbeiten können. und wenn irgendwann mal zeit ist, kann der alte code überarbeitet werden.

mein paradigma ist eigentlich immer: minimalinversiv arbeiten, solange möglich. refaktorierung, wenn nötig. ein guter programmierer sollte sich niemals wiederholen

hustbaer

Klar Refactoring, was sonst?

Code ins Source-Control, damit nix verloren geht/man nachgucken kann was man geändert hat
Alles löschen was auskommentiert ist (ist ja im Source Control erhalten)
Alle "NOP" Statements löschen wie das if das du gezeigt hast
Variablen u. Funktionen die schlecht benannt sind umbenennen (dazu musst du natürlich ein wenig rumsuchen um draufzukommen was die wirklich bedeuten/tun)
Globale Variablen die zusammengehören zu Strukturen zusammenfassen (die dann natürlich erstmal weiter global instanziert werden - ist aber übersichtlicher/verständlicher so)
Funktionen/Klassen "rausfaktorisieren"

Nach jeder Änderung kann man einen oder besser mehr Testläufe machen, und gucken ob die neue Version immer noch die gleichen Ergebnisse ausspuckt wie die Originalversion. Und natürlich überall wo du bei irgendwas ehemals unverständlichem draufgekommen bist was es tut: Kommentar schreiben.

Dadurch solltest du ganz gut verstehen was das Programm macht - auch wenns lange dauert. Wenn du Glück hast kommst du dann drauf dass du im Original bloss ein paar Zeilen ändern musst. Wenn du kein Glück hast wird's halt aufwändig. Wo du die Änderung machst (Original oder neuer Code) kannst du dir aber immer noch aussuchen - je nachdem wie schwer oder einfach es jeweils ist, und wie sehr du deinem Refactoring vertraust.

Bashar

hustbaer schrieb:

Nach jeder Änderung kann man einen oder besser mehr Testläufe machen

Das ist der springende Punkt: Manche Programme sind ohne Refactoring nicht oder nur sehr schwer zu testen. Und Refactoring ohne Tests ist Selbstmord. Ich empfehle an dieser Stelle, mal einen Blick in Michael Feathers: "Working effectively with legacy code" zu werfen.

rüdiger

hustbaer schrieb:

Nach jeder Änderung kann man einen oder besser mehr Testläufe machen, und gucken ob die neue Version immer noch die gleichen Ergebnisse ausspuckt wie die Originalversion. Und natürlich überall wo du bei irgendwas ehemals unverständlichem draufgekommen bist was es tut: Kommentar schreiben.

Gerade das ist doch der kritischste Punkt! Würde mich wundern, wenn er fertige Tests für die Software hat. Ich würde also als aller ersten Punkt wohl die Suche von geeigneten Test-Cases aufnehmen. Aber hier wird es ja schon extrem problematisch, da das Wissen hinter der Entwicklung verloren gegangen ist und man so vermutlich Test-Cases übersieht. Man steht eben vor der Gefahr Fehler zu wiederholen.

Ich denke mal das ein Modul besonders problematisch ist, da man hier leicht (undokumentierte) Quirks in der Modul-Schnittstelle haben kann. Besonders wenn die Software die das Modul lädt eine eigen Entwicklung ist oder nur einen kleinen Kreis von Anwendern hat.

Undertaker

megaweber schrieb:

Taurin schrieb:
Hab schon rund 10 Variationen von solchen if-statements gefunden:
if(iCount==2 && Ifbs_sUser.MsId==315)
    iCount=2;
...dass solche Befehle nachher in einer ausgelieferten Version vorhanden

sowas schmeissen viele compiler sowieso raus und geben warnings wie 'code has no effect' oder sowas.
btw: --> http://www.scitools.com/products/understand/cpp/product.php
gibt's auch als 30 tage testversion

hustbaer

Ich spreche nicht von Unit-Tests, ich spreche davon den Output der Simulation bei gleichem Input zu vergleichen. Das sollte relativ einfach sein denke ich mir.
Und es ist zumindest schonmal besser als nix.

rüdiger

hustbaer schrieb:

Ich spreche nicht von Unit-Tests, ich spreche davon den Output der Simulation bei gleichem Input zu vergleichen. Das sollte relativ einfach sein denke ich mir.
Und es ist zumindest schonmal besser als nix.

Ich sprach auch nicht ausschließlich von Unit-Tests. Auch bei einem Vergleich des I/Os braucht man halt gut geeignete Eingangsdaten.

Wenn der Code bereits in einem SCM liegt oder es zumindest eine Log-Datei gibt, lohnt sich vermutlich zumindest die commit-Nachrichten nach Schlüsselworten (ala Bugfix, New Feature) abzugrasen.

Marc++us

Taurin schrieb:

Es hiess, ich solle das "Traffic Modul" umschreiben,

Könnte man diesen Punkt erst mal klären? Was versteckt sich hinter dieser Anforderung?

Erweitern? Code aufräumen? Neue Funktionen? Debuggen?

Abgesehen davon würde ich lieber mal damit anfangen den Code zu tracen, um die Dynamik im Geflecht zu verstehen.

Zwei Prämissen gelten ja auch bei Spaghetticode:

- er funktioniert ja, also ist er schlecht, aber nicht zwangsläufig falsch
- der Programmierer war vielleicht ein Frickler, aber nicht zwangsläufig dumm

Spaghetticode enthüllt seine Dynamik aber nicht auf dem Papier, da man ihm seine Abläufe nicht ansehen kann. Folglich - rein in das Programm, tracen und loggen, um diese Zyklen zu erkennen.

Taurin

Marc++us schrieb:

Könnte man diesen Punkt erst mal klären? Was versteckt sich hinter dieser Anforderung?

Es wird ein nachrichtentechnisches System simuliert, und dafuer braucht man zwangslaeufig Traffic. Diesen Traffic muss man modelieren (statistische Eigenschaften, Realtime- und Nonrealtimeanwendungen, QoS...). Mir wurde hier ein komplett neues Modell vorgelegt. Vielleicht haette ich nicht 'umschreiben' sondern 'im wesentlichen neu schreiben' sagen sollen...

Marc++us schrieb:

- er funktioniert ja, also ist er schlecht, aber nicht zwangsläufig falsch
- der Programmierer war vielleicht ein Frickler, aber nicht zwangsläufig dumm

Ja klar. Die Leute verstehen sehr viel besser als ich, was sie da simulieren und was da technisch hinter steht. Nur die Sache mit dem Programmieren klappt weniger gut...

Ich hab das Problem jetzt ganz anders geloest: Ich hab (feige wie ich bin) um eine andere Aufgabe gebeten. Ich dachte mir, als unbezahlter Praktikant kann ich mir das mal rausnehmen

Aber danke fuer die vielen Tipps Ich geh jetzt mal wieder an die Arbeit.