Aufgabe 6: Quellcodelokalisierung und Quellcodeanalyse

Christian Becker, Michael Bell, Matthias Füssel, Sven Heine, Uwe Knaur

Die Abarbeitung der Aufgabenstellung erfolgte anhand der beiden vorgeschlagenen Traceläufe. wir haben zu jeder Funktion versucht, kurz die entscheidenden Merkmale herauszugreifen ohne nochmehr Papier als ohnehin schon nötig zu produzieren - damit das ganze noch benutzbar bleibt. Ein zu langes Dokument wird schließlich niemand benutzen wollen, wenn er kurz wissen will, was eine Funktion macht.

1 Trace ohne Aufruf der Dialogbox 'Allgemeine Einstellungen'

Hier folgt nun die Auswertung der Funktionen, die bei dem ersten Tracelauf aufgefallen sind.

1.1 m_steerg.cpp

1.1.1 TAdjustmentParameter::TAdjustmentParameter(void)

1.1.2 TSteering::TSteering(void)

1.1.3 TSteering::Initialize(const HWND hwnd,const int mid,const int sensid)

1.1.4 TSteering::Visualising(int a,int b,int c,int d,int e,int f)

1.1.5 TSteering::StartUp(const HWND hwnd,const int mid,const int sensid)

1.1.6 TSteering::LoadMacro(LPSTR makname,LPSTR fn)

1.1.7 TSteering::GetFileLine(FILE* hFile,LPSTR dst,int dstsize)

1.1.8 TSteering::ParsingMacroId(TMacroTag& macro, LPSTR name)

1.1.9 TSteering::ParsingCmd(TCmdTag& cmd,LPSTR pcmd,LPSTR p1,LPSTR p2, LPSTR p3)

1.1.10 TSteering::ParsingCmdParam(LPSTR param)

1.1.11 Zusammenfassung

Was zuerst auffällt sind die Kommentare. Solange sie trivial sind, sind sie schön sauber und teilweise riesengroß vorhanden, soll heißen völlig unverhältnismäßig zur Bedeutung. In dem Moment, wo es nicht mehr nur um Defaultwerte oder triviale selbsterklärende Funktionsrufe geht, fehlen sie fast vollständig.

Der nächste augenscheinliche Fall von Inkonsistenz tritt bei den Rückmeldungen auf. Wieso wird einmal MessageBox benutzt und ein anderes mal SetInfo? Desweiteren fällt auf, daß die MessageBox immer getFocus () als Argument bekommt. Wenn dies immer geschieht und nicht objektgebunden ist, so kann das die MessageBox auch selber tun und belastet damit nicht das Interface. Da es sich wahrscheinlich um eine Toolkitfunktionalität handelt, ist die Frage, ob es sich bei der häufigen Anwendung lohnt einfach eine Wrapperfunktion zu schreiben (ich denke mal nicht).

Extrem aufgefallen ist der unschöne Hack in TSteering::ParsingCmdParam, um die strcmp-Anfragen zu beschleunigen. Soetwas ist extrem gefährlich weil fehleranfällig. Weiterhin unter die Kategorie unschön fällt die Einschubweise. Mal sind es Tabulatoren ein anderes Mal zwei Leerzeichen.

Im Sinne von OO ist es nicht akzeptabel, so viele globale Variablen zu haben. Außerdem ist es nicht besonders toll, um es mal vorsichtig zu formulieren, direkt über die Definition von globalen Variablen auch noch zu Schreiben, daß man sie eigentlich garnicht braucht und dann solche Variablen auch noch als temnporäre Variablen benutzt. Soetwas kann eigentlich nur irgendwann ins Auge gehen.

1.2 m_main.cpp

1.2.1 TMain::TMain(void)

1.2.2 WinMain( HINSTANCE hInstance,HINSTANCE hPrevInst,LPSTR,int cmdShow)

  1. check auf Konfigurationsdatei
  2. Konfiguration falls vorherige Instanz nicht verfügbar
  3. Durcharbeiten der MessageQueue (while (!SetMessageQueue (cnt-)))
  4. Accelerator laden
  5. Messages absetzen und Fenster initialisieren
  6. Timer testen und gegebenenfalls Fehlermeldung machen
  7. Parameter setzen per SetParameters (lt. Kommentar bedeutet das Hardwareinitialisierung)
  8. MessageLoop anstoßen (ist wohl der eigentliche Arbeitsschritt)

1.2.3 CreateMdiClientWindow(HWND hWnd)

1.2.4 DoSize(HWND hWindow)

1.2.5 DoPaint(HWND hWindow)

1.2.6 TMain::BlastStatusLine(HWND hWindow,HDC hDC,int StatusId)

1.2.7 TMain::SetWatchIndicator(long st)

1.2.8 TMain::DrawStatus(HWND hWindow,ATOM hAtom,int InfoId)

1.2.9 TMain::SetParameters(void)

Diese Funktion macht nichts anderes, als aus den globalen Parametern die Struktur Parameters zu bestücken. Es ist auch wieder nix gewesen mit der Objektorientierung. Es gibt keine Kommentare - dies ist hier auch nicht nötig.

1.2.10 DoCommandsFrame(HWND hwnd,WPARAM wParam,LPARAM lParam)

In dieser Funktion werden die Kommandos ausgeführt bzw. überhaupt erst erkannt (sprich interpretiert).

1.2.11 TMain::LoadProfile(void)

1.2.12 TMain::LoadMenuBar(void)

1.2.13 TMain::MessageLoop(void)

Hierbei handelt es sich um eine absolut Kernfunktion der Klasse TMain. Sie wird durch TMain::WinMain ``angeworfen''.

  1. while (GetMessage(...))

    1. if not TranslateMDISysAccel and notTranslateAccelerator

      1. TranslateMessage
      2. DispatchMessage
  2. return 0
Zuerst einmal sind die Rückkehrcodes sinnlos, da es nur null gibt und dann fehlen sämtliche Kommentare. Der Stil mit den Tabulatoren läßt sich eigentlich nur in einem Editor ertragen, der dies automatisch konvertiert. Der auskommentierte Code deutet auf Bastelei hin und fördert nicht gerade das Vertrauen gerade bei der mangelhaften Auskommentierung. Da die Funktionen nicht im Code zu finden sind und auch nicht protokolliert wurden, wurden diese Codeteile auch nicht erreicht, was logisch erscheint, da nichts getan wurde, sind anscheinend auch keine Messages generiert worden. Das Messagehandling ist aber wohl woanders implementiert - evtl. handelt es sich sogar um Win32-API-calls, aber mit den Kenntnissen des Autors in Win32 ist dies nicht zu klären.

1.2.14 MenuSelect(HWND hWindow,WPARAM wParam,LPARAM lParam)

MenuSelect kümmert sich um die Auswahl in den Menüs. Die eventuell auftretenen Nachrichten werden via PostMessage übermittelt.

1.2.15 TMain::SaveProfile(void)

Die Funktion sichert sämtliche Werte, die vom Konstruktor wieder geladen werden. Es wird ausschließlich WritePrivateProfileString benutzt nicht aber WritePrivateProfileInt (wieder eine Inkonsistenz?).

1.2.16 TMain::TMain(void)

1.2.17 Zusammenfassung

Hier gelten die gleichen Kommentare wie für m_steer.cpp. Dazu kommt noch die Entdeckung, daß es Funktionen und Variablen anscheinend gibt, die komplett groß geschrieben wurden. Diese Schreibweise ist gerade in C mit seinem Präprozessor den Macros vorbehalten. Auch wenn etwas wichtig ist, sollte man nicht diese Schreibweise nutzen. Teilweise sind aber schon einige wesentliche Kommentare da, ist nur die Frage von wem sie sind ;-) Eine böser Stilfehler steckt noch in SetWatchIndicator. Die Kaskade von if-Anweisungen muß ja wohl nicht sein.

Ein inhaltliches Problem stellen die gesamten Statusfunktionen dar, die sämtlichst immer auf einen möglichen Shutdown testen. Hier sollte man eine zentral Prüfstelle festlegen oder genau definieren welche Funktionen darauf testen. Es muß nicht sein das jede dieser Funktionen darauf testet, dann die nächste Funktion ruft und die das nochmal tut.

1.3 m_arscan.cpp

Die hier angegebenen Klassen sollen ignoriert werden, was wir hiermit tuen.

1.3.1 SetHKL(THKL& refl,LPCSTR info)

1.3.2 GetHKL(THKL& refl,LPSTR info)

1.4 m_topo.cpp

1.4.1 TTopography::TTopography(void)

Hier werden einige hardgecodete Initialisierungen vorgenommen.

1.4.2 TTopography::Initialize(void)

Es werden mittels GetPrivateProfileString mehrere Variablen aus dem ini-File gelesen

Es wird wieder nicht GetPrivateProfileInt benutzt.

1.5 Zusammenfassung nach erstem Tracelauf

Also als erstes mal was zum Englischen. Wenn man in einem rein lokal begrenzten Umfeld schon alles ins Englische transformieren will, so soll man doch bitte nicht soviele offensichtliche Fehler einbauen. Allerdings muß ich einräumen, daß ich in einem internationalen Projekt letzens diverse Optionen und Variablen ModulXYZ genannt habe und dies erst in der zweiten Version korrigiert habe.

Mit den Kommentaren ist das so eine Sache - bei Trivialitäten sind sie kein Problem aber da und wenn es ernst wird hat man schon verloren, wenn einem auffällt das Ernst wird. Gerade die Algorithmen zur Erfassung von Macros etc. müssen dokumentiert sein, um ihre Korrektheit prüfen zu können, da sie ja sogar Fehlerstabilisierung versuchen. Hier sollte man vielleicht ansetzen, um später schnell Konfigurationsfehler von Programmfehlern unterscheiden zu können.

Ein weiteres Problem ist der Stil der Einrückungen, was aber relativ einfach behebbar sein sollte. Ein viel größeres Problem ist, daß anscheinend keine genaue Interfacedefinition durchgeführt wurde, was dazu führte, das einige Tests doppelt oder noch öfter durchgeführt wurden.

Problematisch bei der Bewertung stellte sich heraus, daß einige Mitglieder der Gruppe so vor allem auch der Schreiber keine Ahnung von der Win32-API haben.

2 Trace mit Aufruf der Dialogbox 'Allgemeine Einstellungen'

Hier liste ich nur noch die Besonderheiten auf, d.h. alle Funktionen, die beim ersten Durchlauf nicht erkannt wurden. Da wir TModalDlg ignorieren sollen bleiben nur noch die zwei Funktionen von m_dlg.cpp übrig. Eine Zusammenfassung spare ich mir deshalb mal.

2.1 dlg_tpl.cpp

2.1.1 TModalDlg::TModalDlg(char *DlgName)

2.1.2 TModalDlg::ExecuteDialog(HWND hWindow)

2.1.3 TModalDlg::Dlg_OnCommand(HWND hwnd,int id,HWND hwndCtl,UINT codeNotify)

2.1.4 TModalDlg::Dlg_OnInit(HWND hwnd,HWND hwndCtl,LPARAM)

2.1.5 TModalDlg::TModalDlg()

2.2 m_dlg.cpp

2.2.1 TMeasurementParam::Dlg_OnInit(HWND hwnd,HWND hwndCtl,LPARAM lParam)

Der Name der Funktion paßt nicht in das übliche Schema des Codes. Die Funktion wird eigentlich nur zur Datenübergabe an ein Objekt von der Klasse TModalDlg benutzt.

2.2.2 TMeasurementParam::CanClose(void)

Diese Funktion prüft, ob die Einstellungen im Dialog möglich sind. Dabei wird das Deviceobhect testhalber konfiguriert. Es handelt sich insgesamt um zwei checkbuttons und drei Integerwerte. Eine Dokumentation wäre dringend angebracht, da hier inhaltliche Fehler passieren könnten. Der Code an sich ist klar, da er einfach ist ;-)

3 Abschließende Zusammenfassung

An dieser Stelle möchte ich nur noch einmal kurz die wichtigsten Sachen zusammenfassen und nicht nochmal breittreten.

  1. die Kommentare fehlen immer dort, wo es kompliziert wird
  2. die Formatierung des Codes läßt zu wünschen übrig
  3. die vielen teilweise sinnlosen globalen Variablen machen die Nutzung von C++ eigentlich überflüssig (soviel Objektorientierung wie hier bekommt man auch mit C hin)
  4. teilweise sind sprachliche Elemente ungeschickt genutzt worden (z.B. diverse gleiche if-Schleifen hintereinander), was den Code unübersichtlich macht
  5. Probleme mit dme Englischen (naja wir haben hier wohl noch ganz andere Probleme)
  6. Rückkehrcodes sind oft nicht eindeutig
  7. es gibt insgesamt drei Möglichkeiten der Fehlermeldung
  8. schlechte Trennung von Oberfläche und Funktionscode (gerade beim Interpetieren problematisch)
  9. da explizit die Funktionsbenamung in der Aufgabenstellung angesprochen wurde, ist dazu festzustellen, daß bis auf die Inkonsistenz von MDI/Mdi und GetPrivateProfileString hier eigentlich keine Auffälligkeiten beim Patienten festzustellen sind ;-D
Als Vorschlag für die Zukunft wäre ein Tracetool evtl. interessant, welches erkennt, wie die Funktionsaufrufe zusammenhängen und die Funktionsaufrufe entsprechend einschiebt. Wenn also aus File A.cpp Funktion B die Funktion C aus File D.cpp aufruft, so könnte es etwa so aussehen:

Über dieses Dokument ...

Aufgabe 6: Quellcodelokalisierung und Quellcodeanalyse

This document was generated using the LaTeX2HTML translator Version 99.2beta6 (1.42)

Copyright © 1993, 1994, 1995, 1996, Nikos Drakos, Computer Based Learning Unit, University of Leeds.
Copyright © 1997, 1998, 1999, Ross Moore, Mathematics Department, Macquarie University, Sydney.

The command line arguments were:
latex2html -no_subdir -split 0 -show_section_numbers /home/michael/SoftSan/trace1.tex

The translation was initiated by Michael Bell on 2001-01-10


Michael Bell 2001-01-10