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.
Zobaczmy, jakie korzyści płyną z takiego przeglądu:
Poniżej znajdziecie bardziej rozbudowany opis każdego z przytoczonych elementów.
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.
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ć.
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.
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.
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.
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.
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.
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ć.
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.
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ół.
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.
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.
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.
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.
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.
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.
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ę.
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.
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.
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.
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.
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.
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.
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.