Code review w pigułce, czyli jak zrobić to dobrze

Nie lubimy być oceniani. Niektórym z nas oceny kojarzą się z powrotem do szkolnych lat, do czasów sprawdzianów, kartkówek i ustnych odpowiedzi, które mogą budzić negatywne emocje. Pracując jako programista, można spotkać się z praktyką znaną jako code review, czasem również określaną jako inspekcja, przegląd kodu.

Polega ona na tym, że kod, który napiszemy, zanim trafi do głównego brancha, jest przeglądany przez drugiego dewelopera zwanego w tym procesie reviewerem. Jak będziecie mieli okazję przeczytać poniżej, praktyka ta potrafi być przydatnym narzędziem wspomagającym rozwój programisty.

Zyski, zyski, zyski…

 Zobaczmy, jakie korzyści płyną z takiego przeglądu:

• większa uwaga poświęcona jakości kodu, który oddajemy do oceny
• wyłapanie błędów
• sprawdzenie czytelności kodu
• sprawdzenie pod względem merytorycznym
• wymiana wiedzy i rozwój
• sprawdzenie kodu pod względem przyjętych praktyk

Poniżej znajdziecie bardziej rozbudowany opis każdego z przytoczonych elementów.

Większa uwaga poświęcona jakości kodu, który oddajemy do oceny

W Internecie czasem można znaleźć zabawną zasadę, która brzmi mniej więcej tak: “Pisz kod tak, jakby osoba, która będzie go czytać, była seryjnym mordercą, który wie, gdzie mieszkasz”. Kod, który wysyłamy do ludzi, w pewnym sensie jest naszą wizytówką. Sama świadomość, że ktoś po ten kod prędzej czy później sięgnie, powinna wzbudzić w nas motywację i chęć wypuszczenia czegoś dobrego i dopracowanego. Powszechnym wśród ludzi zjawiskiem jest chęć bycia odbieranym jako specjalista w swojej dziedzinie. Tutaj nie ma drogi na skróty. Jak to się mówi „jak nas widzą, tak nas piszą” – chcesz być dobry w tym, co robisz, musisz włożyć w to wysiłek.

Wyłapywanie błędów

Czasem zdarza się, że pisząc jakiś fragment kodu, nie weźmiemy pod uwagę pewnych sytuacji lub np. przez nieuwagę nie uwzględnimy, że coś w funkcjonalności, którą piszemy, może generować błędy. Wyobraźmy sobie programistę (niech ma na imię Janek), który pisze jakąś część systemu przez kilka godzin. Po kilku godzinach Janek myślami jest już gdzieś daleko, jego koncentracja w porównaniu do tej z początkowego etapu pracy drastycznie spada.

Niestety Janek pracuje dalej, bo terminy gonią, a czasu na przerwy za bardzo nie ma. Bardzo łatwo wtedy o błąd czy literówkę. W takich sytuacjach spojrzenie świeżym okiem drugiej osoby na kod może pomóc zdecydowanie szybciej wychwycić błędy i zwrócić uwagę na to, co należy poprawić.

Sprawdzenie czytelności kodu

To punkt, który możemy powiązać z pierwszym na liście. Code review jest też dobrym miejscem do sprawdzenia, czy to, co napisaliśmy, jest czytelne dla drugiej osoby. Być może w przyszłości ktoś będzie chciał rozszerzyć naszą funkcjonalność, więc dobrze, żeby napisany kod był zrozumiały. Jeżeli reviewer ma problemy z rozczytaniem napisanego kodu, może to być znak, że warto na to zwrócić uwagę i spróbować napisać coś bardziej przejrzyście.

Sprawdzenie pod względem merytorycznym

Dobrze, żeby review wykonywała osoba, która zna też kwestie biznesowe związane z daną funkcjonalnością. Samo sprawdzenie kodu nie zawsze wystarcza. Kod może być dobrze napisany, zgodnie ze sztuką, ale nie będzie spełniał wymagań biznesowych. Jeśli np. jesteśmy w trakcie wykonywania review funkcjonalności front-endowej, warto spojrzeć na to, czy napisany kod daje efekt zgodny z zawartością makiet.

Wymiana wiedzy w zespole

Dzięki code review w zespole następuje wymiana wiedzy. Przykładowo – bardziej doświadczony kolega jest w stanie zasugerować rozwiązania, które mogą okazać się przydatne również w przyszłości. Dodatkowo komentarz pozostawiony pod napisanym przez nas kodem może przerodzić się w ciekawą dyskusję, która pozwoli spojrzeć nam na pewne kwestie z innej strony. Błędne jest przekonanie, że tylko w przypadku, gdy review robi osoba bardziej od nas doświadczona, jesteśmy w stanie coś z niego wynieść.

Równie dobrze senior może nauczyć się czegoś od juniora. Juniorzy wnoszą powiew świeżości do zespołów. Często ich niczym nieograniczone pomysły stają się inspiracją np. do optymalizacji czy złamania pewnych obowiązujących standardów. Dodatkowo czasem bardziej doświadczeni koledzy, chcąc zoptymalizować kod swoich podopiecznych, szukają rozwiązań, które to umożliwią, co wiąże się z poszerzaniem wiedzy.

Sprawdzenie kodu pod względem przyjętych praktyk

Jeżeli w naszym projekcie mamy przyjęty zbiór praktyk, jakie musi spełniać kod, to podczas przeglądu kodu reviewer ma okazję sprawdzić zgodność z tymi zasadami.

 
Jak podejść do code review?

Wskazówki dla autora kodu

Ilość kodu

Należy zwracać uwagę na ilość kodu, który jest oddawany do review. Im więcej kodu będzie miał do przejrzenia reviewer, tym mniej dokładna może być jego weryfikacja.

Oczywiście nie zawsze da się tego uniknąć. Wszystko zależy od zadania, które wykonujemy. Zmiany w wielu plikach i dużo nowych fragmentów kodu mogą sygnalizować (ale nie zawsze!), że coś poszło nie tak. Błąd może tkwić w złej ocenie złożoności zadania, jak również może być to oznaka źle zaprojektowanej architektury, na przykład, gdy dodanie nowej z pozoru nieskomplikowanej funkcjonalności wymaga zmian w wielu miejscach.

Sprawdzenie kodu, który oddajemy do review

Chodzi tutaj o to, żeby kod oddawany do review był działający, odpowiednio sformatowany oraz żeby prawidłowo przechodził testy. Najlepiej, gdyby zespół wypracował wspólne zasady i zapisał je w odpowiednim miejscu, tak żeby każdy członek zespołu miał do nich dostęp i w każdej chwili mógł je sprawdzić.

Wskazówki dla reviewera

Dyskusja na żywo vs. Komentarze

Tutaj podejście może zależeć od praktyk w zespole lub od indywidualnych upodobań reviewera. Osobiście stosuję podejście mieszane. W sytuacji, gdy mam np. wątpliwości do jakiegoś fragmentu, wolę skonsultować to na żywo i wynik dyskusji zapisać jako komentarz. Dzięki temu ustalone wnioski nie zginą w natłoku innych spraw.

Forma komentarzy

Komentarze sugerujące jakąś zmianę nie powinny wyglądać w taki sposób: „To jest źle zrobione. Popraw.”. Oprócz sugestii zmiany, komentarze powinny nieść ze sobą treść merytoryczną z propozycją podejścia do problemu. Pamiętajmy jednak, że jest wiele sposobów rozwiązania danego zadania i niekoniecznie nasze jest najlepsze. W takich sytuacjach warto napisać komentarz typu: “Podszedłbym do tego w sposób…”, ale jednocześnie dobrze mieć z tyłu głowy, że nie powinniśmy za wszelką cenę chcieć go wprowadzić, jeżeli rozwiązanie autora jest prawidłowe i nie narusza praktyk przyjętych przez zespół.

Pochwały

Podczas review nie można zapominać o pozytywnym feedbacku przy rzeczach, które nam się spodobały w napisanym kodzie. Takie uwagi motywują do pracy i dalszego rozwoju.

W czym robić code review?

To zależy… Gdy mamy do czynienia z małym i nieskomplikowanym fragmentem kodu, możemy wykorzystać do tego przeglądarkę i podgląd Merge Requesta w managerze repozytoriów. Sprawa wygląda inaczej, jeżeli mamy do czynienia z review bardziej rozbudowanej funkcjonalności lub mamy w review np. jakiś task front-endowy, którego wynik wypadałoby porównać z makietami. W takiej sytuacji powinniśmy uruchomić lokalnie sprawdzany kod lub możemy poprosić autora kodu o prezentację funkcjonalności, co nie zawsze jest możliwe.

Rozwiązywanie konfliktów

Nie każdy musi się zgodzić z tym, co napisze reviewer. Czasem może to doprowadzić do konfliktu. Powody mogą być różne – niezrozumienie tego, co chce przekazać przeglądający lub piszący, goniące terminy, różne podejście do rozwiązywania problemów. Nie ma niestety uniwersalnej metody na rozwiązanie każdego sporu. W takich sytuacjach sugeruję omówić wątpliwości z autorem kodu na żywo, a nie tylko ograniczać się do korzystania z komentarzy. Alternatywą może być dyskusja w większym gronie osób. Czasem spojrzenie trzeciej strony może pomóc rozwiązać sporne kwestie.

 
Elementy poddawane ocenie podczas review

 By wykorzystać review jak najbardziej efektywnie i uniknąć zbędnych konfliktów, warto zachować konkretny standard przyjęty przez zespół. Ułatwi to również wprowadzenie nowych osób. Można do tego podejść na przykład poprzez wprowadzenie checklisty z punktami, na co powinniśmy popatrzeć podczas przeglądu. Takie zasady będą pomocne zarówno dla reviewera, jak i dla piszącego, który będzie wiedział, pod jakimi względami będzie sprawdzany jego kod.

Poniżej znajduje się moja subiektywna lista rzeczy, na które, uważam, że warto zwracać uwagę. Oczywiście zachęcam do eksperymentowania i budowania swojej własnej listy zasad.

Sprawdzenie konwencji nazewniczej i czytelności kodu

Czasem wymyślenie odpowiednio czytelnych nazw dla zmiennych i metod, które będziemy pisać, może być dużo bardziej skomplikowane, niż się wydaje. Jako reviewer warto zwrócić uwagę, czy kod stworzony przez autora jest dla nas czytelny, a użyte nazwy oddają to, co zostało napisane. Oczywiście nie można przesadzać, jeżeli nazwa jest prawidłowa i czytelna, a nam po prostu się nie podoba. W takiej sytuacji można zasugerować zmianę nazwy, ale nie powinien być to powód do niezaakceptowania Merge Requesta i upierania się przy swoim.

Sprawdzenie kodu w IDE

Jest to punkt związany trochę z wymienionym w poprzedniej sekcji fragmentem na temat tego, gdzie przeglądać napisany kod. Tak jak wspomniałem wyżej, jeżeli mamy do czynienia z bardziej rozbudowanym kodem lub mamy wątpliwości co do tego, co zostało napisane, warto sprawdzić go w IDE z odpowiednimi wtyczkami do statycznej analizy kodu. Polecam np. SonarLint. Często też samo IDE wyłapuje już pewne nieścisłości, których na pierwszy rzut oka mógłby nie dostrzec reviewer.

Reużywalność kodu

Czasem, gdy robimy review nowym osobom, można zauważyć, że “wynajdują one koło na nowo”, na przykład nie znają jeszcze całego projektu i nie wiedzą, że można użyć istniejącej już w innym miejscu metody. Dzięki takim uwagom autor kodu będzie wiedział, gdzie na przyszłość szukać, co zaoszczędzi mu czasu, a nam ułatwi review. Zamiast analizować nowo napisaną metodę, będziemy widzieć, że autor użył znanego już nam fragmentu, który dobrze spełnia swoją rolę.

Rozszerzalność

Warto rzucić okiem, czy napisany kod jest łatwo rozszerzalny. Może nawet będziemy kojarzyć, że w następnej kolejności czekają zadania, które będą rozwijać nowo napisany fragment kodu.

Sprawdzenie testów

Nie można pomijać sprawdzenia testów i ograniczenia się tylko do tego, że wszystkie świecą się na zielono. Należy sprawdzić, czy testy zostały napisane prawidłowo i faktycznie dobrze testują utworzony fragment.

Formatowanie kodu

Napisany kod powinien być ładnie sformatowany oraz zgodny z przyjętymi w zespole regułami formatowania. Tu mała wskazówka dla piszących. Warto wyrobić sobie nawyk formatowania kodu w IDE przed zrobieniem commita. Oczywiście nie można przesadzać i nadmiernie zwracać uwagę na pojedyncze spacje i tym podobne drobne rzeczy.

Wpływ na cały projekt

Czasem stworzona funkcjonalność może mieć wpływ na cały projekt. Jako reviewer warto popatrzeć, czy nowo dodany kod nie ma negatywnego efektu na działanie całości. Czymś, co powinno zwrócić naszą czujność, mogą być zmiany w istniejących już testach. Jeżeli na przykład jesteśmy w zespole, który tworzy mikroserwis kontaktujący się z innymi mikroserwisami, źle napisany kod może spowodować ich awarie lub nieprawidłowe działanie.

Zwrócenie uwagi na usuwany kod

Reviewer nie powinien patrzeć tylko na nowo powstałe linijki kodu. Należy zwracać również uwagę na to, co jest usuwane, czy nie ma to wpływu na inne miejsce w projekcie. Czasem może być to zrobione przez nieuwagę. Jeżeli mamy wątpliwości, dobrze jest skonsultować tę kwestię z autorem.

Sprawdzenie merytoryczne

Istotny punkt. Kod może być napisany dobrze, ale co z tego, jeżeli będzie on niezgodny z założeniami biznesowymi. Tutaj przydaje się wiedza z danej domeny. Być może osoba pisząca nie wiedziała o pewnych przypadkach, które należy uwzględnić. Jeżeli i my mamy wątpliwości, robiąc review, warto je skonsultować z kimś, kto dobrze zna domenę, w której działamy, na przykład z zespołowym analitykiem biznesowym.

Słowo na koniec

 Jak widać, praktyka code review niesie ze sobą wiele korzyści. Osobiście dużo się dzięki niej nauczyłem, zarówno od strony reviewera, jak i autora, który oddaje kod do review. Dzięki review mamy możliwość poznać podejście innej osoby do rozwiązywanego problemu. Być może podczas pisania nie zwracamy uwagi na pewne niuanse, które mogą na przykład w przyszłości utrudniać rozbudowę naszego kodu lub generować trudne do wykrycia błędy.

Zachęcam do zwrócenia uwagi na tę praktykę i ewentualne wprowadzenie jej, jeśli nie stosujecie sprawdzenia kodu w swoim zespole.