Kürzlich hatten die M&F Software Entwickler einen Workshop zum Thema Code Review. Im Workshop wurden verschiedene Code Review Techniken diskutiert, um bei M&F eine möglichst hohe Softwarequalität zu garantieren. Die wichtigsten Learnings haben wir hier für Sie zusammengefasst.

Über den Referenten 

Im interaktiven Workshop war die Überzeugung des Referenten Prof. Oliver Augenstein der HSR (Hochschule für Technik Rapperswil) für Code Reviews deutlich zu spüren. Dabei konnten wir über Code Reviews im Speziellen, aber auch über Softwareentwicklung im Allgemeinen sehr viel von der Erfahrung von Prof. Augenstein von seinen Tätigkeiten bei SwissLife und der IBM Research and Development GmbH lernen.

Einleitung

Mit Code Reviews kann die Softwarequalität verbessert werden und Fehler noch vor dem Testen entdeckt werden. Dabei ist entscheidend, dass Code Reviews häufig andere Fehler aufzeigen als automatisierte Tests und sollten daher als Ergänzung eingesetzt werden. Richtig angewendet können Code Reviews die gesamten Entwicklungskosten reduzieren, da Fehler früher entdeckt werden und der Testaufwand verringert wird.

Code Review Techniken

Während des Workshops wurden mehrere Techniken um ein Review zu Organisieren vorgestellt. Diese unterscheiden sich wesentlich im benötigten Aufwand. Daher sollten sehr stark formalisierte und dementsprechend aufwendige Reviews nur an kritischen Stellen eingesetzt werden. Auch empfiehlt sich eine Evaluation über beispielsweise die Anzahl der Findings pro für das Review benötigter Stunden (von allen beteiligten Personen).

Over-the-shoulder review

Workflow: Autor erklärt dem Reviewer die implementierte Änderungen

Dokumentation: Keine

Vorteile: 

• Wenig Aufwand

Nachteile: 

• Probleme wenn Autor über mehr Autorität verfügt als Reviewer
• Autor lenkt Aufmerksamkeit auf die von ihm erwarteten Bedingungen
• Autor verliert sich schnell im Detail

Pair programming

Workflow: Zwei Programmierer entwickeln Code zusammen

Dokumentation: Keine

Vorteile: 

• Expertise ist doppelt verfügbar
• Bei Bedarf für kritische Code Passagen

Nachteile: 

• Zeitaufwendig

E-Mail pass-around review

Workflow: Autor sendet Review Paket per Email

Dokumentation: In Versionsverwaltung

Vorteile:

• Findings werden dokumentiert
• Dünner, informeller Prozess
• Checklisten sollten verwendet werden

Nachteile: 

• Mehrere Reviewer finden den selben Fehler → mehr Aufwand
• Möglicherweise lange Diskussionen (Forum)

Formal inspection (Fargan review)

Workflow: Aufwendiger Prozess mit Introductory, Inspection und ggf. Verification Meeting

Dokumentation: Review Report mit Severity of Error

Vorteile: 

• Checklisten sollten verwendet werden

Nachteile: 

• Sehr zeitaufwendig (Meetings)

Fargan Code Review

Während des Workshops wurde ein Fargan Review anhand von ca. 400 Zeilen C++ Code eines Trainees geübt. Das Fargan Review ist eine sehr formelle und dementsprechend aufwendige Review Technik und sollte daher nur an kritischen Stellen durchgeführt werden.

Ablauf beim Fargan Review

Im kurzen, ca. 15 minütigen Introductory Meeting stellt der Autor das ausgewählte Code Paket vor. Anschliessend wird der Code einzeln von den Reviewers und Readers durchgearbeitet. Im Inspection Meeting (ca. 1 Stunde) werden die Findings zusammengetragen, wobei aus Zeitgründen keine Lösungsansätze diskutiert werden sollten.

Rollen beim Fargan Review

• Moderator: Organisiert Meetings, hält Prozess am Laufen und unterbindet ausufernde Diskussionen
• Recorder: Dokumentiert Findings während des Inspection Meetings in einem Review Report
• Reader: Stellt Code Zeile für Zeile während des Inspection Meetings vor (ist ebenfalls Reviewer)
• Autor: Stellt generellen Code im Introductory Meeting vor und beantwortet Fragen während des Inspection Meetings (darf nicht gleichzeitig Moderator, Recorder oder Reader sein)
• Reviewer: Versucht Fehler im Code zu finden

Erfolgskriterien

Unabhängig von der Art des Reviews gibt es Faktoren die entscheidend für die Qualität des Reviews sind. Diese werden in den folgenden sechs Code Review Regeln zusammengefasst.

Dokumentiere Findings

Die gefunden Fehler sollten sauber dokumentiert und hinterlegt werden, damit keine Findings verloren gehen und Wiederholungen vermieden werden.

Beschränke Einfluss des Autors

Der Autor stellt generell den zu erwartenden Standardfall vor und ignoriert mögliche Sonderfälle. Unter Umständen wird sogar versucht Fehler gezielt zu verstecken. Daher sollte der Autor die Reviewer nicht zu stark beeinflussen. Umgekehrt sollten sich die Reviewer der exponierten Position des Autors bewusst sein und das Review nicht nutzen, um sich zu profilieren.

Benutze Checklisten

Mit Checklisten kann die Anzahl der Findings gesteigert werden. Ausserdem können Checklisten eingesetzt werden, um gezielt bestimmte Aspekte im gesamten Team zu verbessern (z.B. kann eine Checkliste über Standards in der Dokumentation vorgegeben werden). Weiterhin führt ein kontinuierlicher Einsatz von Checklisten dazu, dass Autoren auf die in der Checklist aufgeführten Arten von Fehler sensibilisiert werden. Dies führt zu einer langfristigen Steigerung der Code Qualität und macht die Checklist ggf. irgendwann überflüssig.

Kleine Portionen reviewen

Reviews sind anstrengend und deutlich produktiver, wenn kleine Code Portionen reviewed werden. Daher sollte nicht mehr als 60 Minuten am Stück reviewed werden und um möglichst viele Fehler zu finden, sollten nicht mehr als 200-400 Zeilen Code pro Stunde reviewed werden.

Mehrmals Lesen

Es empfiehlt sich den Code mehrmals zu lesen. Bei einem ersten, schnellen Lesen geht es um das generelle Verständnis der einzelnen Funktionen und Abschnitte. Beim wiederholten Lesen können Checklisten verwendet werden um gezielt nach Fehlern zu suchen.

Evaluiere Review

Gerade aufwendige Reviews sollten im Anschluss evaluiert werden, um den Kosten-Nutzen Aufwand abzuschätzen. Daher empfiehlt sich eine Dokumentation der Findings und der aufgewendeten Zeit des Autors, der Reviewer und aller anderen beteiligten Personen. Dies beinhaltet die Zeit für die Vorbereitung, das Review aber auch für Meetings im Zusammenhang mit dem Review.

Design

Im zweiten Teil des Workshops wurde etwas informeller über Software Design gesprochen. Dabei sind einige interessante Ansätze und Probleme besprochen worden.

State

Eine häufige Fehlerquelle ist es, dass der State des Objektes nicht korrekt überprüft oder angepasst wird innerhalb einer Methode. Insbesondere kann es vorkommen, dass Unklarheit über den State eines Objektes herrscht, weil dieser durch eine Reihe boolescher Felder ausgedrückt werden, welche nicht orthogonal zu einander sind.

Hier sieht man, dass das Objekt 4 States haben kann (Kombination der 2 Flags). Man sieht aber auch, dass einer dieser States nie erreicht werden sollte: (!isInitialized && isRunning). In so einem Fall wäre es besser, ein Enum einzuführen für die States des Objektes. Damit würde man auch explizierter zum Ausdruck bringen, was man abzubilden versucht.

"Fail Often and Early"

Software die fehlerhaft ist, sollte möglichst früh im Entwicklungsprozess und möglichst nahe an der Fehlerquelle den Fehler zeigen. Wird darauf nicht geachtet, kann es durchaus sein, dass gewisse Stellen mit unbrauchbaren Resultaten weiterrechnen und von aussen den Anschein erwecken, korrekt zu laufen. Wenn der Fehler dann endlich eine sichtbare Auswirkung zeigt, ist es teurer ihn zu korrigieren. Da es einerseits aufwändig ist, sich durch den Code zu kämpfen, bis die Quelle des Fehlers gefunden ist und andererseits der Entwicklungsprozess in der Zwischenzeit voran geschritten ist.

Dies soll verhindert werden, indem die Software möglichst oft 'meckert', wenn etwas nicht stimmt. Eine Funktion sollte also beispielsweise nicht einfach einen 'silent return' ausführen oder NULL zurückgeben, wenn etwas nicht den Erwartungen entspricht. Sondern es sollte dafür gesorgt werden, dass der Entwickler von diesem Problem umgehend erfährt. In der Testumgebung könnte also beispielsweise eine Exception geworfen und der Test abgebrochen werden. In der Liveumgebung könnte die Software entsprechende Logs schreiben und den Benutzer darauf aufmerksam machen, dass eine Funktionalität nicht mehr korrekt läuft.

Preconditions

Eine Möglichkeit früh und oft den Fehler zu bemerken sind Pre- und Postconditions.

Precondition: Eine Funktion überprüft zu Beginn jedes Aufrufes, ob die übergebenen Parameter den Erwartungen entsprechen. Dazu zählt bei Methoden auch der Zustand der Klasse und der Instanz.

Es müssen alle Annahmen überprüft werden, auf welche sich der Code während der Ausführung der Funktion verlassen muss. Allen Entwicklern ist der Test gegen NULL bekannt, bevor eine Referenz dereferenziert wird. Dies lässt sich aber auch verallgemeinern, wenn man beispielsweise prüft, ob eine Zahl einen sinnvollen Wert hat oder wenn der State eines Objektes überprüft wird.

Postcondition: Bevor der Aufruf beendet wird, muss überprüft werden, ob die Funktion gemacht hat, was von ihr erwartet wird. Bei einer Methode wird zum Beispiel geschaut, ob sich der Rückgabewert, die Instanz und die Klasse in dem erwarteten Zustand befinden.
Der Referent merkt hier allerdings an, dass die Postcondition einer Funktion in der Precondition der nächsten Funktion ohnehin überprüft wird und somit nicht zwingend notwendig ist.