Code Reviews im Daily Business

Im Beitrag Was ein Team-Leader für sein Entwicklungsteam tun kann hatte ich das Thema Code Review bereits angesprochen. Als etwas, das ein Team Lead selbst (besser im Team) zu tun hat. Spät aber doch habe ich Code Reviews in meinem Team eingeführt. Stichprobenartig hatte ich selbst bereits davor begutachtet. Dieses Mal sollten die Reviews jedoch als fixer Bestandteil für alle installiert werden. Nun läuft dies seit 1,5 Wochen. Noch keine lange Zeit, aber erste Erkenntnisse sind nun vorhanden.

Was sagt die englischsprachige Wikipedia zum Thema Code Review:

Code review is systematic examination (often known as peer review) of computer source code. It is intended to find and fix mistakes overlooked in the initial development phase, improving both the overall quality of software and the developers’ skills. Reviews are done in various forms such as pair programming, informal walkthroughs, and formal inspections.

Wie alles in der Softwareentwicklung geht auch dieses Thema theoretisch sehr schnell in die Tiefe. Unterschiedlichste Möglichkeiten und Ausprägungen sind möglich. Damit Code Reviews zusätzlich zur täglichen Arbeit längerfristig Bestand haben können, habe ich mich für diese Variante (unabhängig der Theorie) entschieden:

Größere Umsetzungen, die einen mehrstündigen oder mehrtägigen Aufwand nach sich ziehen sollten unbedingt vor dem Check-In begutachtet werden. Kleinere Check-Ins werden täglich zu einem festgesetzten Termin durchgesehen. Dies inkludiert auch Änderungen etwaigen Ressourcen (beispielsweise von Übersetzungen), oder aber auch Projekt- und Solution-Dateien.

Nun, fast zwei Wochen später, ist es Zeit für einen ersten Rückblick.

Durchführung der Reviews

Ja, wie führen wir Reviews eigentlich durch? Den Anfang hat das gemeinsame Zusammensetzung um einen PC gemacht. Danach werden sämtliche Changesets seit dem letzten Review durchgegangen. Ausgelassen wird nichts. Mittlerweile sind wir auf einen Beamer umgestiegen, da dies insgesamt ein wenig komfortabler sind. Der nächste Schritt könnte ein einschlägiges Tool sein, welches Kommentare etc. einfacher zulässt. So sicher bin ich mir hierbei allerdings noch nicht, da das “Zusammensitzen” das Team als solches wesentlich festigt und Ablenkungen minimiert werden. Es gibt kein blinkendes Skype, Lync, keine E-Mail-Benachrichtigungen und was sonst noch so alles aufblinkt.

Positive “Entwicklung”

Ein Hauptargument gegen derartige “Prozesse” ist die fehlende Zeit. Zeit, die besser in die Implementierung von Funktionen investiert werden sollte, sind doch Punkte zeitig abzuschließen. Diese “Tatsache” wurde bewusst von mir ausgeblendet, in der (durchaus naiven) Hoffnung, dass dem nicht so sei.

Zeitaufwand höher als geplant, aber auch der Gewinn

Gerade für die täglichen Reviews war ein Serientermin eingerichtet worden. Täglich 30 Minuten. Gerade am Beginn der Code Reviews hat sich jedoch gezeigt, dass zusammen mit dem Review auch zahlreiche Konzepte genauer inspiziert und erklärt werden müssen, da diese offensichtlich schlecht kommuniziert wurden. Dies kostete wesentlich mehr Zeit als geplant. Teilweise dauern die Review Meetings 1,5 bis 2 Stunden. Das ist sehr viel Zeit. Allerdings muss man dem entgegen halten, dass

  • Konzepte an Hand von Beispielen (den konkreten Implementierungen) vermittelt, verstanden und teilweise auch wiederholt und somit gefestigt
  • durch das neue Verständnis, nachfolgende Umsetzungen wesentlich schneller und – vor allem auch – korrekter (nach internen Standards) implementiert

werden. Es entsteht für jedes Team-Mitglied ein klareres Bild der gesamten Anwendung, wodurch mehr Entscheidungen selbständig und in besserer Qualität getroffen werden können.

Kultivierung von Standards / Guidelines

Wenn Softwareentwickler zusammen treffen ist das Ergebnis meist mit Arztbriefen zu vergleichen. Gehe zu vier Ärzten und erhalte fünf Meinungen. Unsere Code Reviews haben gezeigt, dass über Implementierungen, aber auch Standards (Guidelines) ausgiebig diskutiert wird. Das ist eine wirklich gute Sache. Die Gedanken des werten Lesers könnten aber auch durchaus in diese Richtung gehen:

Standards und Guidelines werden vorgegeben und jeder hat sich daran zu halten.

Durchaus legitim. Funktioniert aber so nicht. Ein Guideline-Dokument muss gelesen werden (ja, man kann es auch durchgehen, aber ohne konkrete Beispiele ergibt das oft keine Antwort auf die Frage nach dem Warum. Besser, die Standards bzw. Guidelines werden diskutiert und an Hand konkreter Codestellen besprochen. Alleine durch die Diskussion bleibt das neu erworbene Wissen besser erhalten, als stures “Auswendiglernen” (sofern das überhaupt gemacht wird).

Hach, das gibt es doch schon

Code Reviews dienen nicht ausschließlich der Verbesserung des Codes. Vielmehr wird während derartiger Sessions auch Know How transferiert. Da gibt es diese Klasse und jene Methode die für solche Fälle genutzt werden kann. Die Folge sind “Aha”-Erlebnisse und Hinweise auf Codestellen, die darauf umgebaut werden sollten. Daraus entsteht eine Todo-Liste mit konkreten Aufgaben, die bis zum nächsten Meeting umzusetzen sind. Das sind häufig kleinere Änderungen, meist aber mit einer hohen Reduktion an Code verbunden.

Für kommende Implementierungen ist dieses Wissen vorhanden. Code wird nicht dupliziert, sondern eingesetzt. Die Aufgabe ist schneller zu lösen, es wird bereits bewährter (und idealerweise getesteter) Code eingesetzt – und das dauerhaft. Da zahlt es sich schon aus, 1-2 Stunden darüber zu sprechen.

Fehlerhaften Code finden

Bei fast jedem Review ist es uns gelungen Fehler zu finden. Teilweise wären diese recht schwer zu finden gewesen. Es werden zwar Abnahmetests durchgeführt, Teile auch per Unit/Integration Tests behandelt, aber eben nicht alles. Da sind aber noch nicht so weit, dies flächendeckend zu schaffen. Ein Stein nach dem anderen. Um Tests zu kultivieren haben wir eine andere Maßnahme getroffen. Dazu aber ein anderes Mal mehr.

Fakt ist aber, dass uns Code Reviews (für alle nachvollziehbar) helfen, Fehler zu finden und zu beheben. Fehler, die entweder gar nicht auffallen, oder einen zusätzlichen Roundtrip bei der Abnahme verursachen.

Negative “Entwicklung”

Der negative Part (der aber in Wahrheit schon in die positive Richtung geht) war, dass es Anfangs zahlreiche Diskussionen über die Implementierungen gab. Gefühlsmäßig würde ich mal behaupten, dass Kritik am Code in manchen (zugegeben wenigen) Fällen persönlich genommen wurde. In der zweiten Woche hat sich dies wesentlich gebessert. Es ist angekommen, dass der Code und nicht der Erzeuger im Vordergrund steht.

Es ist vollkommen nebensächlich wer fehlerhaften Code verursacht hat. Dieser ist nun mal eben vorhanden und muss richtig gestellt werden.

Zusammen mit dieser “Erkenntnis” drehen sich Diskussionen plötzlich um das tatsächliche Problem und einer möglichen Lösung, anstatt um Rechtfertigungen.

Fazit

Ich kann jedem empfehlen Code Reviews als festen Bestandteil zu etablieren. Noch sind wir mit dieser Phase nicht durch. Wir sind aber auf einem guten Weg und die bisherigen Ergebnisse sprechen eindeutig dafür. Es gibt tatsächlich weniger Fehler und Wissen wird beständig im Team verteilt. Aktuell müssen zwar zahlreiche Stellen korrigiert und zusammengeführt werden. Das kostet ein wenig Aufwand, erhöht aber die Stabilität ungemein und reduziert dabei die Komplexität. Äußerst positiv für die Zukunft.

Veröffentlicht von Norbert Eder

Ich bin ein leidenschaftlicher Softwareentwickler. Mein Wissen und meine Gedanken teile ich nicht nur hier im Blog, sondern auch in Fachartikeln und Büchern.

Schreibe einen Kommentar

Deine E-Mail-Adresse wird nicht veröffentlicht. Erforderliche Felder sind mit * markiert

Cookie-Einstellungen
Auf dieser Website werden Cookie verwendet. Diese werden für den Betrieb der Website benötigt oder helfen uns dabei, die Website zu verbessern.
Alle Cookies zulassen
Auswahl speichern
Individuelle Einstellungen
Individuelle Einstellungen
Dies ist eine Übersicht aller Cookies, die auf der Website verwendet werden. Sie haben die Möglichkeit, individuelle Cookie-Einstellungen vorzunehmen. Geben Sie einzelnen Cookies oder ganzen Gruppen Ihre Einwilligung. Essentielle Cookies lassen sich nicht deaktivieren.
Speichern
Abbrechen
Essenziell (1)
Essenzielle Cookies werden für die grundlegende Funktionalität der Website benötigt.
Cookies anzeigen