Das Codereview: was still kaputtgehen kann

Die Sicherheitsprüfung fragt, ob jemand an Daten kommt, die ihm nicht zustehen. Das Codereview stellt eine andere Frage: Ist der Code so gebaut, dass ich ihn in einem Jahr noch verstehe, ändern kann und mich auf ihn verlassen kann? Beide liefen am selben Tag im selben Prüflauf, mit derselben Regel, dass jeder Befund eine Fundstelle braucht. Hier geht es um die Hälfte, die mit Struktur, Tests und Dokumentation zu tun hat.

Der wichtigste Fund war kein Code

Das Dringendste, was das Review gefunden hat, war ein Prozessfehler. Die automatische Prüfung, die bei jeder Änderung Formatierung, Codestil, Typen, Tests und den Bau durchläuft, war auf dem Hauptzweig rot. Vier Dateien waren nicht richtig formatiert. Das klingt harmlos, aber die Formatierungsprüfung stand als erster Schritt, und nach dem ersten Fehler bricht der Lauf ab. Alle wirklich wichtigen Prüfungen danach liefen also gar nicht mehr.

Aufgefallen war das niemandem, auch mir nicht. Lokal war alles grün, und in der Übersicht stand ein roter Haken an einem Schritt, der nach Kosmetik aussieht. Dort sucht man keinen abgeschalteten Testlauf. Ich habe die vier Dateien formatiert und die Formatierungsprüfung ans Ende gestellt. Ein Fehler in der Formatierung soll nie wieder verhindern, dass die Tests laufen.

Kommentare, die lügen

GODAV ist ungewöhnlich ausführlich kommentiert, und viele Kommentare erklären, warum etwas noch nicht da ist. Das Review fand vier solcher Kommentare, die längst falsch waren. Einer behauptete, die Anmeldung über Microsoft sei noch nicht gebaut, obwohl das Modul fertig war. Ein anderer sagte, es gebe im Datenmodell weder Zeiterfassung noch Risiken. Hätte jemand diesem Kommentar geglaubt, hätte er nie eine Kennzahl zur Zeiterfassung im Dashboard ergänzt.

Ein veralteter Kommentar ist gefährlicher als gar keiner, weil er gelesen und geglaubt wird. Ich hätte eine automatische Regel einbauen können, die nach Wörtern wie „noch nicht” sucht. Die Suche ergab aber über fünfhundert Treffer, fast alle ganz normale deutsche Sätze. Stattdessen gibt es jetzt eine Regel für das Schreiben: Wer einen Kommentar über etwas Künftiges schreibt, nennt die Stelle, an der es nachgeliefert wird. Dann findet ihn, wer dort später arbeitet.

Die stillen Fallen

Am meisten Zeit kosten in einem Projekt die Fehler, die nicht abstürzen. Ein paar davon hatte ich in den Wochen davor bereits erlebt, und das Review hat geprüft, ob sie wirklich ausgeräumt sind.

Die bekannteste ist Kalendertag gegen Zeitpunkt. Ein Zieltermin wie der 30. September ist ein Tag ohne Uhrzeit und ohne Zeitzone. Liest man ihn versehentlich wie einen Zeitpunkt in der Ortszeit, wird westlich von Greenwich der 29. daraus. In Deutschland fällt das nie auf, weil die Uhr hier vor der Weltzeit liegt. Deshalb läuft der Test dafür absichtlich in der Zeitzone von New York: In Berlin wäre er auch mit dem Fehler grün.

Ein Zieltermin ist ein Tag, kein Zeitpunkt. Die Kalenderwochen zählen nach ISO, wie in Zeiterfassung und Berichten.

Eine andere Falle saß in den Filtern. Ein Haken, der auf „aus” stand, kam beim Server als „an” an, weil eine Umwandlung vor der eigenen Prüfung lief. Am Aufgabenboard blieb der Filter für Spalten mit finalem Status deshalb ohne Wirkung. Auch hier gibt es heute einen Test, der genau unter den Bedingungen läuft, unter denen der Fehler entstand.

Ein Filter, der die falsche Hälfte zeigt, meldet keinen Fehler. Man merkt es nur, wenn man genau hinsieht.

Was gut war

Ein Bericht, der nur Mängel aufzählt, wird beiseitegelegt. Deshalb hält das Review auch fest, was bleiben soll:

• Die Regeln, welche Schicht welche andere kennen darf, sind nicht nur beschrieben, sondern werden beim Prüfen des Codestils automatisch erzwungen.
• Im gesamten Code steht kein vergessener Merkzettel, keine Debug-Ausgabe und keine Datei, die nirgends verwendet wird.
• Das Skript, das den Rückstand der Testläufe aufräumt, löscht nur, was ein Test erkennbar selbst angelegt hat, und zeigt vorher beim Namen, was entfallen würde.

Offen geblieben sind kleinere Punkte, etwa eine Klasse, die mehr Aufgaben trägt als die übrigen und eigentlich aufgeteilt gehört. Sie stehen im Bericht, und der nächste Lauf vergleicht mit diesem.