Ein “kluger” Satz über Code Reviews

von Nils Langner am 11. Juni 2010

Lesetipps

Vier Tage Konferenz haben gereicht, um einen klugen Satz über Code Reviews mit heimzubringen. Ich weiß zwar gar nicht, ob er so gemeint war, wie ich ihn verstanden habe, aber trotzdem mache ich mal nen Artikel draus und philosophiere ein wenig drüber. Ich sollte den Satz erstmal loswerden, bevor ich zu viel schreibe, oder?

Mit Code Reviews reparierst du nicht den Code, sondern den Entwickler!

Klingt im Deutschen natürlich nicht so fließend wie im Englischen, egal der Sinn wird trotzdem klar. Warum ich den Satz so toll finde ist einfach. Früher und damit meine ich vor einer Woche, war ich mir sicher, dass man Code Reviews hauptsächlich macht, um fiese Stellen im Code zu finden und sie zu beseitigen. Damit sollte der Code besser werden. Das stimmt natürlich, aber das ist vielleicht gar nicht der Hauptvorteil.

Das erste was passier, wenn man Code Reviews ankündigt ist, dass der Entwickler sich viel mehr Gedanken macht, was man denn so in die Tastatur knallt. Es schaut ja schließlich jemand drüber Das Review an sich hat dann noch mal einige der Vorteile, die Pair Programming bietet. Man kommuniziert über den Code. Und wie beim Pair Programming ist es wohl so, dass mit der Zeit beide Entwickler auf ein ähnliches Niveau kommen. Zum Glück ist es das Niveau des besseren.

Wahrscheinlich lag das schon immer auf der Hand diese Erkenntnis, aber ich finde der Satz bringt einem das nochmal wunderbar und einprägsam ins Gedächtnis. Deshalb ist es auch mein Satz des Tages und einen Artikel wert.

Nils Langner

Auch wenn Ihr es mir nicht glauben werdet, aber ich habe nichts gegen PHP. Ich rege mich einfach nur gerne auf. Ok so schlimm ist es auch nicht. Eigentlich wollte ich schon immer einen Blog haben und da ...

Zum Profil von Nils Langner

14 Kommentare »

Norbert Bartels
am 11. Juni 2010 um 07:43 Uhr

Den Satz finde ich wirklich gut. Und es beschreibt ja genau das was passiert. Der Entwickler wird durch das Review besser und produziert danach besseren Code – zumindest im Idealfall.

Also kann man den Kreis ziehen zu der Aussage, dass der Code durch Code Reviews besser wird.
red
am 11. Juni 2010 um 08:40 Uhr

Naja, kommt schon sehr auf die Qualität des Reviews an. Je nachdem kann der Schuss auch nach hinten losgehen und am Ende hat man nur noch einen demotivierten Mitarbeiter..
Ralph Meier
am 11. Juni 2010 um 09:22 Uhr

Ein wahrer Satz von einem klugen Menschen.

Mich würde mal die Praxis von Reviews interessieren. Wie macht ihr das und in welcher Intensität?
- wieviel Zeit wendet der Reviewer für die Vorbereitung auf?
- was wird kontrolliert?
- darf der Codeauthor noch etwas erklären oder muss der Reviewer alles selber verstehen?
- wie lange wird der Code verbessert?
- …
Mich interessiert einfach die Vorgehensweise. Ich bemerke nämlich immer wieder bei mir, dass ein Review einige Zeit benötigt und wenn ich wenig Zeit habe, dann mache ich zwar ein Review bemängle aber nur die Benennung von Variablen und sage vielleicht noch, dass hier der Code nicht so verständlich ist. Verstanden habe ich den Code dann aber nicht …. was ist das Ziel des Reviewers?
ragtek
am 11. Juni 2010 um 09:25 Uhr

Naja, beim Pair Programming lernen ja indirekt beide dadurch.
Deswegen denke ich nicht dass der Schuss nach hinten geht, das meiner Meinung nach entscheidende ist “die Stimmung” zw. den Entwicklern.
Wenn diese passt, wird keiner von beiden demotiviert.

=> Der Entwickler wird durch das Review besser und produziert danach besseren Code – zumindest im Idealfall.
:::::::::::::::::::::::

Ja, ein sehr wichtiger Punkt (Im Idealfall)
Leider ist es nicht immer so, aber meine Erfahrung hat gezeigt, dass wenn beide Coder (oder wir haben das teilweise auch zu 3. gemacht) für Code Reviews offen sind (sind ja def. nicht alle, manche können/wollen keine Kritik) man da “immer” dazulernt
Christian
am 11. Juni 2010 um 09:35 Uhr

Mich würde zudem interessieren, wie man seine Vorgesetzten davon überzeugt, dass man Code-Reviews und auch XP bzw. Pair-Programming braucht?

Ich muss immer wieder folgendes beobachten:
Man kommt in eine Firma, man sieht Missstände (die “alten” Entwickler sehen das genauso) und hat Vorschläge wie eben XP oder einzelne Teile daraus, aber sie werden abgeschmettert. “Kostet zu viel Zeit, die wir in Weiterentwicklung stecken können.” Wobei man “Weiterentwicklung” besser durch Bugfixing ersetzen müsste. Aber das verstehen die irgendwie nicht. Auch nach mehreren Jahren in der Firma werden die Hinweise und Vorschläge nicht ernst genommen.
Nun kommt ein alter Bekannter (Entwickler) der Obrigkeiten aus vergangenen Zeiten – zu meist als Consultant – und schlägt Pair-Programming, Code-Reviews und UnitTests vor. Und plötzlich werden diese Dinge eingeführt.

Kommt das jemandem bekannt vor oder treffe ich immer auf die falschen Firmen/Cheffs?
Ingo
am 11. Juni 2010 um 09:49 Uhr

… und wer hat den Satz gesagt?!?
ragtek
am 11. Juni 2010 um 10:08 Uhr

Ja, kommt mir sehr bekannt vor;)

War auch einer der Gründer, wieso ich meine damalige Arbeit(genauer gesagt Lehrstelle) aufgegeben habe.
Ich konnte einfach nicht mit meinem Chef umgehen.
Er war nicht offen für neues, beharre auf sein “Know How” das doch ein Jahrzehnt alt ist (also nichts mit OOP, OOP ist böse und dauert viel länger, Also eigentlich alles was man heutzutage als gut ansieht zB Automatisierte Tests, etc verfluchte er,… darunter leide ich heute noch da das “riesige Wissenslücken” sind)…
Da es mehrere Unstimmigkeiten mit ihm gab und ich dadurch nicht zufriedem mit meiner Arbeit war, aber mir das Programmieren (und die Leidenschaft) durch “ihn” nicht versauen wollte, ging ich.
Sven
am 11. Juni 2010 um 10:14 Uhr

Also ich interpretiere den Satz auch noch etwas weiter. Weil der Reviewer repariert ja nicht den Code im gesamten Projekt, sondern gibt dem Entwickler eben auch Hinweise wo er was besser machen kann. Auf diese Weise kommt der Entwickler auch schneller auf ein höheres Level und man kann ihm schlechte Angewohnheiten schneller austreiben.

Und ein Review muss nicht immer manuell sein. Ein CI-Server der checkstyle oder PMD hat und die Ergebnisse dann im Projekt öffentlich sichtbar macht zeigt auch allen schnell wo sie Dinge verbessern können.
ragtek
am 11. Juni 2010 um 10:18 Uhr

“Also ich interpretiere den Satz auch noch etwas weiter. Weil der Reviewer repariert ja nicht den Code im gesamten Projekt, sondern gibt dem Entwickler eben auch Hinweise wo er was besser machen kann. Auf diese Weise kommt der Entwickler auch schneller auf ein höheres Level und man kann ihm schlechte Angewohnheiten schneller austreiben.”

Geht es nicht genau darum? Genau so hab ich den Satz auch interpretiert.

Und ja, automatisiert werden dir deine vorher festgelegten Code Standard Regeln ausgebessert, aber die Logik und die “eigentliche Kunst hinter dem Programmieren” usw kann IMHO nur von Mensch zu Mensch weitergegeben werden.
Sven
am 11. Juni 2010 um 10:45 Uhr

Na ich hatte Nils so verstanden, dass er meinte dadurch, dass man den Review ankündigt agiert der Entwickler einfach vorrausschauender…
hakre
am 11. Juni 2010 um 13:05 Uhr

Ich habe keine Ahnung wie der Satz im Original heisst, aber von “Kaputten Entwicklern” zu sprechen finde ich ein wenig unschön.

Warum spricht man nicht von Weiterbildung?
Jens Reinartz
am 11. Juni 2010 um 14:20 Uhr

Codereview ist bei uns in der Firma zwar schon seit längeren ein Thema, allerdings wirkten die sporadischen Review-Meetings sperrig und unflexibel.

Das Problem war u.a., dass der Autor und Reviewer zeitnah einen Termin finden mussten; dieser organisatorische Wasserkopf hat vor allem abgeschreckt.

Wir konnten das Problem durch ein Webtool (http://smartbear.com) lösen. Dieses (in Zusammenarbeit mit SVN) erlaubt das einfache Hochladen von Änderunge. Der Reviewer kann nun schnell (aber ohne seine Arbeit unterbrechen zu müssen) den Review durchführen. Das Webtool zeigt die Änderungen als Diff an und ermöglicht es so dem Reviewer, schnell die Änderungen zu beurteilen. Über ein eingebautes Chattool kann der Reviewer direkt mit dem Autor kommunizieren und so problematische Codezeilen besprechen.

Mich würde interessieren, wie Ihr vor geht?
stietze
am 11. Juni 2010 um 15:38 Uhr

@ragtek

Ebenso ist mein Chef auch. Objektorientierte Programmierung ist für ihn derzeit leider nix anderes als eine Zusammenfassung von Funktionen. Und dass das unnötig sei.

Die Vorteile sieht er leider nicht.

Zum Thema CodeReviews: machen wir hier in der Firma leider auch nicht, wäre auch gar nicht möglich, denn die total konfuse Programmierung würde in jedem Fall ein Refactoring des ganzen Projektes benötigen.

Bzw. wir machen schon eine Art Code-Reviews (ich – also der Auszubildende – mit dem Chef) – aber dabei zeige ich ihm immer wieder, warum wir jetzt für die Entwicklung eines neuen Features für ein Groß-Projekt doppelt so lange brauchen und obendrein noch zig Fehler produzieren, weil es einfach nicht mehr überschaubar ist.

Und ich werde in meinem Potenzial total ausgebremst.
Latita
am 12. Juni 2010 um 18:03 Uhr

@ragtek @stietze OMG und Beileid. Ich hab vergleichsweise lang gebraucht um OOP zu verinnerlichen, aber ist es nicht tödlich, sich als Entwickler per se dagegen zu stellen?

Bei uns ist CodeReview jedenfalls ein wichtiges Thema und finde das auch sehr gut. Es hindert einen daran, zu überheblich mit dem eigenen Code zu werden. Was funktioniert, ist nicht gleich guter Code ^^