Spagetticode verstehen?

Taurin

Ich bin hier gerade (als Praktikant) in einer nachrichtentechnischen Arbeitsgruppe gelandet und habe eine kleine Programmieraufgabe bekommen. Zuerst dachte ich jedefalls, sie waere klein. Es hiess, ich solle das "Traffic Modul" umschreiben, bin dann aber ueber folgene Probleme gestolpert:

- Das alte "Modul" verteilt sich ueber die vollen 10k Codezeilen der Simulation
- Alle wichtigen Funktionen haben hunderte Codezeilen (Rekordhalter 1,5k LOC)
- Es gibt doppelt so viele globale Variablen wie Funktionen
- Es gibt im wesentlichen keine Doku oder Kommentare
- Dafuer aber viel alter, auskommentierter Code
- Und viele kleine, programmiertechnische Absonderlichkeiten

Hinzu kommt, dass ich nur rudimentaer mit Nachrichtentechnik vertraut bin. Ich verstehe zwar, was ich programmieren soll, aber ich bin nicht mit den Details der gesamten Simulation vertraut.

Habt ihr ein paar Tips, wie man sowas sytematisch durchdringen kann?

Ich komme mir ein bischen vor wie bei thedailywtf

Taurin schrieb:

Habt ihr ein paar Tips, wie man sowas sytematisch durchdringen kann?

1. Mach den Leuten klar, dass der Code (schlecht) schwierig zu handlen ist.
2. Mach den Leuten klar, dass die vermeintlich kleine Aufgabe durch den Code sehr aufwändig wird.
3. Trete (wenn möglich) mit den Autoren in Kontakt. Die können dir Fragen noch am ehesten beantworten.
4. Fang an Kaffee zu trinken. Wenn du schon Kaffee trinkst, trink mehr davon

Shade Of Mine

und um himmelswillen bloß nicht versuchen zu refaktorieren.

Bashar

Willkommen im Wahren Leben(TM).

Toa

Bashar schrieb:

Willkommen im Wahren Leben(TM).

Du hast ja so recht ! .. refaktorieren bedeutet doch einen Code komplett umschreiben bis er hinsichtlich Style und funktionalität angepasst ist oder ist das falsch ? Bin Leider erst 10te Klasse xD und Lehrer sagen immer frag nach wenn du ein Wort nicht verstehst ^^

Shade Of Mine

Toa schrieb:

Du hast ja so recht ! .. refaktorieren bedeutet doch einen Code komplett umschreiben bis er hinsichtlich Style und funktionalität angepasst ist oder ist das falsch ? Bin Leider erst 10te Klasse xD und Lehrer sagen immer frag nach wenn du ein Wort nicht verstehst ^^

Im Prinzip ist es korrekt. Wobei die Funktionalität nicht geändert wird (zumindest theoretisch nicht, praktisch sieht es da wieder anders aus ;)), nur die interne Aufteilung.

Man würde zB den Codes des OPs durch Refaktorierungen in einen schönen OO-Code umwandeln. Allerdings bräuchte man dazu einiges an Zeit. Und Zeit ist Geld. Und das Geld muss wo her kommen. Die Firma wird es nicht zahlen und so bleibt alles an dem OP hängen. Wenn ihm langweilig ist kann er natürlich alles wunderschön refaktorieren - aber den meisten Leuten ist in der Arbeit nie langweilig genug für so ein Unterfangen.

Taurin

Shade Of Mine schrieb:

und um himmelswillen bloß nicht versuchen zu refaktorieren.

Ich werde mich hueten! Je mehr ich mir den Code anschaue, desto verwunderter bin ich, dass er tatsaechlich funktioniert...
Hab schon rund 10 Variationen von solchen if-statements gefunden:

if(iCount==2 && Ifbs_sUser.MsId==315)
    iCount=2;

Am besten werde ich versuchen, hier mal ne sinnvolle Aufgabe zu bekommen, statt mit so einem Quatsch Zeit zu verschwenden...

megaweber

Taurin schrieb:

Hab schon rund 10 Variationen von solchen if-statements gefunden:
if(iCount==2 && Ifbs_sUser.MsId==315)
    iCount=2;

Das sieht mir nach einem miesen Debugversuch im Falle von iCount == 2 aus .... wenn man mal überlegt, dass solche Befehle nachher in einer ausgelieferten Version vorhanden

Taurin

Wird ja gluecklicherweise nie Ausgeliefert werden. Ist ja Simulation, und das einzige, was da 'ausgeliefert' wird, sind Simulationsergebnisse. Macht es aber auch nicht besser.

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.