100% Code muss gereviewt werden?
von Maud Schlich
Heute fragt mich jemand:
Wir möchten von der Vorgabe “100% Code muss gereviewt werden” runterkommen. Allerdings mit nachvollziehbaren Kriterien. Kennst Du Ansätze, wie dies in der Industrie typischerweise gehandhabt wird?
Ja, da gibt es sogar mehrere.
1) Messen und bewerten
Für alle Codereviews messen, was dabei herauskommt (dazu folgt demnächst ein eigener Blogeintrag) und natürlich den Aufwand.
Wichtig aber, dass auch die “Umgebungskriterien” mit gelistet werden. Und dann alle Codereviews in drei Kategorien einteilen:
A) Gut, dass wir das Codereview gemacht haben, das war den Aufwand wert, hätten wir sonst nie gefunden.
B) Ok, wir haben ein paar wichtige Sachen gefunden, hätten wir ziemlich sicher auch mit Hilfe von Tests gefunden und war den Aufwand bedingt wert, immerhin haben wir was bei gelernt, ein paar grundlegende Verbesserungen sind dabei rumgekommen, die Wartbarkeit wurde verbessert.
C) Das hat nichts gebracht außer Aufwand. Keiner der gefundenen Fehler war für zukünftige Projekte oder für Wartbarkeit relevant, das Review-Team war so klein/erfahren/…, dass niemand besonders viel gelernt hat.
Dann analysieren welche Umgebungskriterien mit der Einteilung A/B/C korrelieren und C ganz streichen, A in jedem Fall reviewen und dann noch soviel B hinzufügen, bis die 80:20 Regel erfüllt ist –> siehe Punkt 3)
2) Abhängig von der zyklomatischen Komplexität nach McCabe
Nur der Code wird gereviewt, der eine zyklomatische Komplexität > x hat. Mc Cabe selbst sagt x = 10, ich meine, dass hängt extrem von der Programmiersprache ab und die objektive Komplexität sollte ebenfalls bewertet werden. Bei der zyklomatischen Komplexität sollte unbedingt ein “Chef-Entwickler” eine Schnellprüfung machen und ggf. dieses Komplexitätsmaß “überstimmen”.
3) 80:20 Regel
Nur 20% des Codes wird gereviewt bzw. der Code, der 20% des Systemverhaltens bestimmt bzw. zu den statistisch gesehenen 20% am häufigsten aufgerufenen / durchlaufenen Anwendungsteilen zählt.
4) Risikobasiert
Auswahlkriterien sind:
- Code von Neulingen (entweder im Programmieren überhaupt oder in der Branche oder auch in der jeweiligen Stelle) - Code, der voraussichtlich eine hohe Wiederverwendbarkeit haben wird (–> Bibliotheken) - Code, der eine hohe Kritikalität für die gesamte Anwendung bzw. das gesamte System hat - Code, der sicherheitsrelevante Funktionen des Systems realisiert (der SIL-Level bestimmt dann die Intensität des Reviews – z.B. SIL-Level >= 3: formale Inspektion mit min. 2 Gutachtern + Moderator + Autor) - Code, der in der architektonischen Hierarchie ganz unten steht und eine hohe Aufrufwahrscheinlichkeit hat BTW: wenn man das sauber macht, dann bestimmt die Risikoanalyse auch die Intensität und Skalierung der Testfallentwurfsverfahren, die angewendet werden und entsprechend gut zu den Reviews passen.
5) Entscheidung des gesamten Projektteams bei festen Planungsvorgaben.
Zeitpunkt: Beim ersten Vorhandensein von Anforderungsdokumenten und dann verfeinert bei jeder Stufe des Designs. Prinzip: “Töpfchen und Kröpfchen” – es gibt ein festgelegtes Kontingent an Zeit und Budget für alle Code-Reviews und ein Durchschnittswert für verschieden Reviewtypen für verschiedene Codestücke (meist nur anhand von LOC) und deren Anwendungsort (z.B. im eigentlichen Steuergerät oder “nur” in der Diagnose-SW, Firmware oder “nur” Auswertungs-SW für den PC). Das Team entscheidet dann, welche Codestücke formal gereviewt werden und welche nicht. Aufwand und Budget müssen dabei “restlos” verbraucht werden. Hier kenne ich es oft, das jeder Code nur einem – eher formalen – Walkthrough unterzogen wird, keine Inspektion. Heißt: Autor ist Moderator, das Protokoll besteht aus einer To-Do-Liste auf dem Flipchart + einem handschriftlich kommentierten Ausdruck des Codes.
Und was nun nehmen?
Je nachdem, welche Metriken schon vorhanden, bzw. wie leicht diese zu erheben sind, empfehle ich eine Kombination aus 1) und 4).
Zusätzlich würde ich die genutzten Reviewtypen variieren. D.h. die ganze Bandbreite von Inspektion über Walkthrough bis zu informellen Reviews (Schreibtischtests) ausnutzen, aber in jedem Fall wenigstens ein kurzes Durchführungsprotokoll und eine minimale Anzahl an Kennzahlen erheben (also nicht alle Fehler protokollieren bei Walkthroughs und informellen Reviews, aber z.B. eine Strichliste für die Fehler nach Schwere + Aufwand protokollieren lassen).
Aber auch die Variante 5) mag ich persönlich gerne in Kombination mit 4).
4) ist ohnehin mein absoluter Favorit für alle Formen von statischen und dynamischen Tests – merkt man vermutlich auch ;)
Alle diese Varianten habe ich in der Industrie gesehen oder auch selbst so eingeführt.
Einen Kommentar schreiben