Dobre Code Review

Wytwarzanie oprogramowania, szczególnie na większą skalę oraz w długim terminie, wymaga utrzymywania jakości kodu na możliwie jak najwyższym poziomie. Jako programiści mamy wiele różnych praktyk, które nas w tym wspierają. Jedną z nich jest tytułowe Code Review. Trzeba jednak zauważyć, że każda osoba wykonuje Code Review w inny sposób. W tym wpisie opiszę swój sposób na przeglądanie kodu oraz postaram się wyodrębnić cechy, które według mnie określają dobre Code Review.

Korzyści płynące Code Review

Na początku wyjaśnijmy sobie, co zyskujemy dzięki poświęceniu czasu na przeprowadzenie Code Review. Podstawową zaletą jest zwiększenie jakości kodu w projekcie oraz wychwycenie potencjalnych problemów lub błędów jeszcze na etapie implementacji.

Jednak korzyści z Code Review czerpią także sami programiści — zarówno osoba, która oddała swój kod do sprawdzenia, jak i osoba kod sprawdzająca. Czytając, dyskutując na temat konkretnych rozwiązań oraz wprowadzając poprawki, stale się rozwijamy. Każdy ma szansę nauczyć się czegoś nowego. Jest to szczególnie istotne w przypadku osób rozpoczynających swoją karierę lub świeżo zatrudnionych pracowników. Czytanie kodu osób bardziej doświadczonych i obeznanych w projekcie pozwala szybciej wejść na wyższy poziom podczas realizacji własnych zadań. Dlatego wyznaję zasadę, że każda osoba w zespole powinna móc przeprowadzać Code Review, niezależnie od stanowiska czy doświadczenia.

W tym miejscu może pojawić się pytanie: “A co jeżeli jestem młodszym programistą i nie znajdę żadnego błędu? Czy nie będzie to zmarnowany czas?“. Jeżeli w trakcie czytania kodu nauczyłeś się chociaż jednej rzeczy, to nie jest to czas zmarnowany, a wręcz inwestycja dla całego zespołu.

Warto też zaznaczyć, do czego Code Review nie służy. Nie ma tutaj miejsca na udowodnianie, że osoba, która napisała kod jest gorsza. Programowanie to zwykle praca zespołowa i naszym zadaniem jest pomaganie, a nie wytykanie błędów. Jeżeli oddany kod jest naprawdę słabej jakości, to powinniśmy dołożyć wszelkich starań, żeby kolejne zmiany wychodzące spod ręki tej osoby były tylko lepsze.

Zanim przejdziemy do Code Review

Kiedy już chcemy spojrzeć na wprowadzone zmiany, wstrzymajmy się na moment i zadajmy sobie kilka pytań. Czy znamy zakres oraz założenia zadania? Czy wiemy jakbyśmy sami do tego podeszli? Jakie rozwiązanie przychodzi nam do głowy? W przypadku większych zadań warto poświęcić 10-15 minut, przeczytać treść zadania, obejrzeć makietę lub inną dokumentację i zastanowić się przez chwilę. Pozwoli nam to na dokładniejsze spojrzenie w kod. Bez tego ciężko wychwycić przypadki krańcowe, błędy logiczne czy braki w funkcjonalności. Czasami uda nam się także wymyślić kontrpropozycje do zaproponowanego rozwiązania.

Osobiście lubię też “przeklikać się” przez wprowadzone zmiany na uruchomionej aplikacji. Uzyskujemy wtedy perspektywę użytkownika, a to przekłada się także na inny sposób myślenia o implementacji. Pomaga to też zrozumieć, dlaczego autor wybrał konkretne rozwiązanie problemu.

Oczywiście nie popadajmy w skrajności. Jeżeli dobrze znamy zmieniany kod albo jest tych zmian naprawdę mało, to możemy pominąć szczegółowe przygotowania i bezpośrednio przejść do komentowania.

Rodzaje komentarzy

Niektórym osobom Code Review kojarzy się wyłącznie z uwagami i wymaganymi poprawkami. Komentarze zawierające wyłącznie uwagi powodują, że cały proces staje się na dłuższą metę frustrujący. Od tego już tylko jeden krok do zupełnej niechęci przed wystawianiem zmian do sprawdzenia.

Żeby temu zapobiec, proponuję rozszerzyć zakres komentarzy o kilka nowych kategorii. Moja klasyfikacja wygląda w następujący sposób:

• uwagi — komentarze dotyczące miejsc, które wymagają poprawy. Mogą to być uwagi dotyczące jakości kodu, odstępstw od konwencji projektowych, błędów funkcjonalnych itd., które zdaniem osoby sprawdzającej należy poprawić. Oprócz samego wskazania problemu warto także dodać sugestię, w jaki sposób dany problem można rozwiązać
• uwagi NTH (Nice To Have) — to sposób podpatrzony od pewnego seniora (pozdrawiam, jeżeli to czytasz) z mojej pierwszej pracy. Podczas sprawdzania kodu często mamy myśli typu “zrobiłbym to inaczej, ale w sumie aktualne rozwiązanie też jest ok”. Właśnie wtedy z pomocą przychodzi uwaga rodzaju NTH. Możemy podzielić się naszym pomysłem, ale decyzja o jego wprowadzeniu zależy od chęci i dobrej woli autora kodu. Taką uwagę można też zastosować, gdy zauważymy miejsce na dobry refactor, a nie leży on już w zakresie zadania
• pytania/dyskusje — w przypadku gdy nie potrafimy zrozumieć zachowania jakiegoś kodu, warto rozpocząć dyskusje. Jest to o wiele lepsze niż przemilczenie, bo kiedyś może przyjdzie nam ten kod zmieniać. W przypadku świeżych programistów to może być też dobra metoda na uzyskanie dodatkowej wiedzy od bardziej doświadczonych kolegów. Dyskusje mogą być prowadzone także na tematy wykorzystanych bibliotek lub innych narzędzi
• pochwały — jeżeli zauważymy ciekawy wzorzec, świetny algorytm, porządnie zaimplementowany komponent, to napiszmy o tym coś miłego. Takie komentarze pozwalają utrzymać miłą atmosferę i zachęcają do lepszej pracy

Iteracyjne Code Review

Po dodaniu komentarzy oraz otrzymaniu odpowiedzi wraz z nowymi zmianami powinniśmy przejść do kolejnej iteracji. Kod, który zmienił się od ostatniego sprawdzenia, powinien zostać przejrzany. Jeżeli zmiany są zadowalające, osoba, która dodała komentarz, powinna go zamknąć. W przeciwnym razie powinna pojawić się informacja wraz ze wskazówkami, co jeszcze trzeba zrobić. Można dodawać również nowe komentarze.

Taki cykl powinien powtarzać się aż do rozwiązania wszystkich uwag oraz zamknięcia dyskusji. Pamiętajmy jednak, że jeżeli kod został już raz zaakceptowany, to nie dodawajmy tam na nowo uwag. Powoduje to zwykle frustracje u autora, a czasem nawet interesariuszy, który czekają z niecierpliwością na nowe funkcjonalności. Dlatego pamiętajmy, że iteracja dotyczy wyłącznie nowych zmian.

Nie jestem zwolennikiem oznaczania komentarzy jako zamkniętych przez autora zmian. Wyklucza to iteracyjne podejście i pozwala na wprowadzenie do projektu niesprawdzonego kodu.

Akceptacja zmian

Częstym powodem do dyskusji jest sprawa dotycząca liczby osób, która musi zaakceptować zmiany, żeby uznać je za gotowe do wdrożenia. Myślę, że to zależy od tego, jak rozbudowane jest to zadanie, czego ono dotyczy oraz jak duży jest zespół. Nie ma tutaj jednej odpowiedzi.

Moim zdaniem zmiany o dużym znaczeniu lub dotykające wielu miejsc w projekcie warto, żeby przeglądały dwie osoby, w tym przynajmniej jedna z większym doświadczeniem. Dla pozostałych przypadków nie widzę przeszkód, żeby swoją zgodę wyraziła tylko jedna osoba.

Unikałbym też sytuacji, w których Code Review jest przeprowadzane przez trzy lub więcej osób. Zwykle powoduje to zbyt wiele różniących się od siebie sugestii, czasem nawet sprzecznych. Dodatkowo czas poświęcony na przeprowadzenie Code Review znacznie się wydłuża, a jakościowo zysk jest znikomy.

Pamiętajmy, że akceptując zmiany, niejako podpisujemy się pod tym kodem i bierzemy za niego część odpowiedzialności. W końcu po zapoznaniu się z nim stwierdziliśmy, że nadaje się do naszego projektu i spełnia wszystkie nasze standardy.

Podsumowanie

Sprawdzanie kodu to poważne zadanie na równi z pisaniem własnego kodu. Mogę nawet posunąć się do stwierdzenia, że jest to praca trudniejsza. Czytanie cudzego kodu wymaga więcej wysiłku i skupienia. Praca ta jednak zawsze zwraca się w długim terminie, dlatego warto wykonywać ją jak najlepiej.