Code review - najfajniejszy kawałek procesu ;-)

Lubię code review.
Jako reviewer i jako autor. Choć to drugie mniej ;-)

Czemu lubię?

Największa zaletą code review jest to, że mamy kogoś kto patrzy na kod z boku, bez skazy wynikłej z godzin spędzonych nad zrozumieniem jak to wszystko działa.
Widzi wydestylowaną zmianę, bez wcześniejszych nieudanych prób.
To świeże* spojrzenie pozwala dostrzec miejsca gdzie coś jest niejasne, albo te miejsca gdzie coś można poprawić.

Bo gdy coś jest niejasne "to wiedz, że coś się dzieje...."

Czemu coś jest protected? Albo czemu coś zmieniło się na public?
A czy to dziedziczenie jest potrzebne?
O! A tutaj można by było użyć buildera, albo strategii i byłoby jaśniej.

Autor po godzinach dotykania się z danym kawałkiem kodu ma jakiś mentalny model zmiany, ale przez to może nie dostrzegać w nim dziur, albo miejsc gdzie można by go było uprościć.

Dodatkowy bonus to to, że code review pozwala się uczyć.
O tym jak wygląda kod, ale poznawać różne sztuczki** z przybornika innych developerów.

Jest jeszcze ta rzecz, że podchodząc do kodu trzeba chcieć go pokonać, nie tą osobę która kod napisała, ale sam kod, trzeba próbować na nim "ataków" przez przekazywanie null'a, pustych list, wartości spoza zakresu, albo wołania w kilku wątkach.
Gdy robi się review testów dobrze najpierw spojrzeć czy dany test w ogóle coś testuje.
Tutaj przydaje się IDE i zmienienie czegoś w kodzie tak by sprawdzić czy jakiś test na tym wyleci...

Jak coś znajdziemy autor może nas przez jakiś czas nienawidzić ;-) ale jeśli dzięki temu kod będzie mniej zaskakujący tym lepiej...

Na co uważać?
Na code smellsy, na nadmierne używanie abstrakcji, na zbyt luźne poczynanie sobie z dostępem do danych i metod, na premature optimatization i na to czy kod robi daną rzecz najprościej jak się da (z zastrzeżeniem, że nie jest to kawałek, który specjalnie był pisany by być szybkim i wiemy, że musi taki być (bo np. prosty algorytm to O(N²), a bardziej skomplikowany to O(log N))).
Nazwy zmiennych i metod też są ważne, ale mniej, ważne by nie myliły, nie muszą wcale być zbyt opisowe, część ludzi gubi się przy nazwach składających się z kilku słów, szczególnie gdy wiele z nich jest podobnych.


* - dlatego wg mnie zawsze powinno być kilkoro reviewerów i nie powinno być tak, że znaczące jest tylko słowo kogoś kto zna dany fragment kodu.
** - jak brudne to jeszcze lepiej ;-)


Podobne posty^beta
SCRUM i ogólnie Agile to często taka zakamuflowana forma premature optimization
Może mi ktoś wytłumaczyć po co istnieją GWT i GWTP?
Promptem w ToDo ;-)
Statyczna analiza kodu, czy to ma sens?
4 dniowy tydzień pracy - idea piękna... ale obawiam się, że tylko zwiększy rozwarstwienie