Dobre praktyki Code Review — poradnik początkującego

Jak zrobić dobre Code Review? Od czego zacząć? O czym pamiętać? Jakich błędów się wystrzegać? Jakich narzędzi CR użyć? Ten artykuł odpowie na wszystkie te pytania, a może nawet sprawi, że w pełni zrozumiesz, że efektywna inspekcja kodu potrafi zdziałać cuda nie tylko w samym produkcie, ale i zespole programistów.

Transparent Data
Blog Transparent Data
8 min readMay 19, 2020

--

Code Review w (krótkiej) teorii

Code Review to praktyka stosowana obecnie w prawie każdej firmie, zajmującej się rozwijaniem oprogramowania. Pozwala wykrywać błędy na wcześniejszym etapie cyklu CI/CD i zmniejszać ryzyko ich późniejszego wystąpienia. Stanowi także doskonałą okazję do wymiany wiedzy między programistami — zarówno wiedzy na temat dobrych praktyk kodowania, jak i wiedzy dziedzinowej danego produktu.

Definicyjnie CR wyjaśnia się najczęściej następująco:

Code Review (w skrócie CR, po polsku inspekcja kodu, przegląd kodu lub czytanie kodu) — to praktyka polegająca na przeglądaniu kodu napisanego przez programistę przez drugą osobę (najczęściej kolegę z zespołu) w celu zmniejszenia ryzyka wystąpienia błędów oraz dostosowania go do przyjętych w danej firmie standardów i założeń związanych z architekturą.

Bardziej po łebsku: “Zanim coś wjedzie na produkcję, pokaż swój kod komuś z zespołu, a on go sprawdzi czy wszystko git”.

I to tyle w teorii, jaką możemy znaleźć w Internecie.

Code Review w rzeczywistości

W rzeczywistości można się spotkać z różnymi formami przeprowadzania Code Review. W osobnym artykule możecie przejrzeć konkretne przykłady dobrze i źle wykonanego Code Review w kodzie.

Jedni wykonują je bardzo dokładnie, “czepiając się” każdej nazwy zmiennej czy metody i “oburzają się“ na widok niepotrzebnego komentarza. W takich miejscach pracy bardzo często dochodzi do sporów między programistami próbującymi nakłonić innych do swojej racji. Finalnie, psuje to morale zespołu i ogólną atmosferę.

Inni traktują Code Review jako przykry obowiązek i konieczność odhaczenia kolejnego checkboxa, żeby tylko jak najszybciej wydać następną wersję produktu. Tutaj też przegląd kodu nie działa tak, jak powinien.

Aż w końcu znajdujemy firmy, które stosują praktyki Code Review po to, aby czerpać z nich jak najwięcej korzyści. Używają go do wdrażania nowych programistów w istniejący kod. Dzielą się wiedzą, umożliwiając mniej doświadczonym programistom osiągnąć wyższy poziom. Pilnując siebie nawzajem przed używaniem złych rozwiązań, dbają o nie zaciąganie długu technologicznego. Ostatecznie, poprawiają też relacje między członkami zespołu, tworząc przyjazne i rozwojowe środowisko pracy.

Różnice między tymi odmiennymi podejściami nasuwają zasadniczo dwa pytania, na które odpowiemy w dalszej części artykułu:

  • Jak zadbać o to, aby nasze Code Review było pozytywnym wydarzeniem zarówno dla nas, jak i dla drugiego programisty?
  • Jak przeprowadzić skuteczne i efektywne Code Review?

Dobre praktyki Code Review

Zasada nr 1: Mniej znaczy więcej

Przeglądanie zbyt dużej ilości kodu “na raz”, w jednym czasie, jest niezwykle męczące. Gdy skupiamy się długo nad jednym fragmentem, nasza wydajność spada, a ryzyko przepuszczenia błędu rośnie. Dlatego, zdecydowanie lepiej jest oddawać do przeglądu małe fragmenty kody. W ten sposób robienie CR tak nie męczy.

Przeprowadzono już wiele badań, z których płynie jeden wniosek:

Mały commit — > Krótkie review — > Lepszy kod

Nie należy rozumieć tego opacznie! Krótkie Code Review wcale nie oznacza, że powinniśmy się ścigać z kolegami, kto szybciej przejrzy jak największą liczbę linii kodu!

Aby znaleźć dobry punkt odniesienia, za dobry wynik możemy przyjąć średnie opracowane przez Google, z ich case study Modern Code Review. Jeżeli chodzi o liczbę przeglądanych plików:

  • 35% Code Review dotyczyło zmian w tylko 1 pliku,
  • 90% zmian nie wykraczało poza 10 różnych plików.

Oczywiście liczba plików nie jest do końca dobrą miarą, jeżeli pracujemy w środowisku, gdzie jeden plik może mieć kilka tysięcy linii kodu. Dla osób mających do czynienia z takim długim kodem, lepiej jest przyjąć średnią liczbę linii kodu oscylującą w okolicach 24 (dane z tego samego badania Google).

Średnie są świetnym wzorcami dobrych praktyk, ale warto mieć też w świadomości, że nawet w Google znajdują się działy, gdzie mediana linii kodu na jedno Code Review liczy sobie 250. To się po prostu zdarza.

Zasada nr 2: Checklista Code Review

Aby proces robienia Code Review był zawsze kompletny i nic kluczowego nie umknęło naszej uwadze, niesamowicie przydaje się sporządzenie sobie listy najważniejszych rzeczy do sprawdzenia w kodzie. Najlepiej będzie, gdy przyjmie ona formę checkboxów “do odhaczenia”.

Całkiem fajne przykłady checklist CR bez problemu znajdziecie w Sieci (np. TUTAJ lub TUTAJ). Warto je jednak potraktować tylko i wyłącznie jako pierwszą wersję naszej własnej listy, którą z czasem będziemy modyfikować i dostosowywać do swojej obecnej sytuacji.

Jeśli nasz kod na starcie jest w dużym nieładzie, nie mamy w nim testów, a co więcej, bez większej pracy nie jesteśmy w stanie takich testów wprowadzić, nie ma co tworzyć checklisty z 50 punktami, gdzie uwzględnimy wszystkie dobre praktyki stosowane przez innych. W takiej sytuacji, gdy za każdym razem połowa punktów checklisty nie może być spełniona i możemy w ich miejsce postawić tylko minusy, taka checklista zadziała demotywująco i już na początku naszej przygody zniechęci nas do dalszej pracy.

Na początek, znacznie lepszym rozwiązaniem będzie krótka lista, która podniesie jakość naszej pracy, ale i nie sparaliżuje nas ilością negatywnego feedbacku.

Przykład krótkiej checklisty CR dla początkujących:

  1. Czy rozumiem kod, który czytam?

1.A. Czy nazwy zmiennych są poprawnie dobrane?

1.B. Czy metody są dobrze nazwane i sparametryzowane?

2. Czy kod jest zgodny z ogólnie przyjętymi standardami kodowania?

3. Czy kod realizuje to co jest opisane w zadaniu?

Powyższa prosta checklista pozwoli nam w szybki i łatwy sposób rozpocząć pracę nad Code Review i poprawianiem kodu. Już po kilku iteracjach będziemy mogli zobaczyć, że nasz kod zaczyna wyglądać trochę lepiej — zgodnie z zasadą skauta, robimy porządki po trochu. A kiedy już dojdziemy do wprawy i wejdzie nam w nawyk wystrzeganie się tych podstawowych błędów, możemy zacząć rozszerzać tę listę o kolejne punkty.

Przykładowe, kolejne punkty, do dodania do naszej checklisty CR w kolejnych iteracjach:

  1. Czy kod spełnia zasady SOLID?
  2. Czy nie ma w nim zbędnych duplikacji?
  3. Czy dobrze zarządzamy wyjątkami?
  4. Czy odpowiednie testy zostały rozpisane?

Jak już wspomnieliśmy, to tylko przykładowe punkty. Konstrukcja takiej checklisty powinna być wynikiem wspólnej pracy zespołu i co ważne, powinna być listą systematycznie aktualizowaną wraz z rozwojem i poprawą naszego kodu oraz naszej własnej wiedzy.

Zasada nr 3: Narzędzia do Code Review

Mówi się (nie bez powodu), że dobry programista, to leniwy programista, a to dlatego, że kiedy nie chce mu się czegoś zrobić samemu, to zawsze znajdzie proste rozwiązanie, które go zastąpi w pracy lub chociaż mu w niej pomoże. Ta zasada dotyczy także Code Review i choć nie jesteśmy jeszcze w stanie całkowicie wykluczyć człowieka z tego procesu, to istnieje już sporo narzędzi, których możemy użyć, żeby ułatwić sobie przegląd kodu.

Celem tego artykułu nie jest omawianie wszystkich dostępnych narzędzi, ale o trzech poniższych (triggerach, statycznej analizie kodu i standardach kodu), naprawdę warto wiedzieć.

Triggery

Aby zautomatyzować procesy związane z Code Review poprzez używanie gotowych narzędzi, musimy mieć możliwość automatycznego ich uruchamiania. W tym temacie pomóc nam mogą rozwiązania typu git hooks oraz bitbucket pipelines, czyli narzędzia wbudowane w nasze repozytoria kodu.

Jeśli z jakichkolwiek powodów nie możemy, bądź nie chcemy ich użyć, naprzeciw wychodzą nam takie rozwiązania, jak Jenkins, Drone.io czy Travis.

Statyczna analiza kodu

W celu ułatwienia sobie monitorowania kodu, możemy poddać go procesowi statycznej analizy.

Dzięki temu procesowi, otrzymamy różnego rodzaju informacje, takie jak złożoność cyklomatyczna, łączna liczba linii kodu w projekcie czy średnia liczba linii kodu w pliku. Dane te pozwolą śledzić nam rozwój naszego systemu i reagować w momencie, gdy zaobserwujemy negatywny trend.

Na przykład, jeśli średnia liczba linii kodu w plikach z wydania na wydanie ciągle nam rośnie, może to być znak, że nie pracujemy odpowiednio nad podziałem kodu na małe klasy. Małe klasy są wszak łatwiejsze w utrzymaniu i ich późniejszym testowaniu.

Standardy kodu

Jeśli w naszej firmie, cały zespół pracuje (lub stara się pracować) zgodnie z pewnym ustalonym standardem kodowania, to istnieje też narzędzie, które automatycznie przypilnuje, żeby nic, co nie spełnia naszego standardu, nie trafiło na naszą wersję produkcyjną. Takim narzędziem dla języka PHP jest dla przykładu PHP CS Fixer, który nie dość, że pozwala sprawdzać poprawność standardów, to jeszcze jest w stanie dostosować kod do raz zdefiniowanego standardu.

Zasada nr 4: Spraw, by miało to znaczenie

Ostatni punkt, ale chyba najważniejszy ze wszystkich. Niezależnie od wybranych narzędzi, przyjętych standardów i ilości poświęcanego czasu, proces Code Review i jego cele muszą być dobrze zrozumiane w całym zespole programistycznym. Odpowiedzialność za to ciąży na wszystkich osobach — zarówno na tych, które sprawdzają kod, jak i na tych, które go napisały.

Obowiązkiem osoby sprawdzającej kod nie jest tylko i wyłącznie odhaczenie checkboxów na liście, którą przygotowaliśmy. Osoba taka powinna również kierować swoje uwagi i zastrzeżenia do kodu w taki sposób, aby jego autor jasno zrozumiał, jaki wpływ ma dana zmiana lub jej brak i mógł się czegoś z takiego feedbacku nauczyć. Tylko krytyka odpowiednio przekazana i konstruktywna wpływa pozytywnie na rozwój drugiego człowieka. Dbałość o ten aspekt polepszy też ogólną atmosferę pracy w zespole. Bo przecież, kto nie chciałby pracować w miejscu, gdzie stale może rozwijać swoją wiedzę i umiejętności?

Z kolei autor kodu musi nauczyć się dystansu do swojej własnej pracy — nie może przyjmować wszystkich uwag personalnie. To niezwykle ważne, aby uwagi, jakie dostajemy w wyniku Code Review, traktować jako możliwość nauki i dostrzeżenia innych możliwych rozwiązań. “Wszystkim nam zależy, aby produkt był jak najwyższej jakości” powinno stanowić absolutny filar każdej inspekcji kodu.

Ważnym elementem jest też dobre osadzenie Code Review w przyjętym w danej firmie procesie wytwarzania oprogramowania. Nie możemy dopuścić do sytuacji, gdy ktoś poświęca dużą ilość czasu na Code Review, wyciągając wnioski i obserwacje mogące polepszyć kod, po czym wiedza ta ląduje w miejscu, gdzie nikt do niej nie wraca. Takie sytuacje wywołują frustracje u osób, które przykładają się do Code Review. W dobrze zorganizowanym procesie, zawsze jest czas na poprawę zaobserwowanych uwag.

Na zakończenie ważna rzecz: Spory, konflikty i odmienne zdania

Ważnym elementem procesu Code Review jest również opracowanie mechanizmu rozwiązywania sporów między programistami, którzy nie potrafią się sami porozumieć. Zaniedbanie tego elementu może ostatecznie doprowadzić do pojawienia się przewlekłych konfliktów i ogólnego pogorszenia atmosfery. Może też sprawić, że całkowicie porzucimy proces Code Review, mimo wszystkich płynących z niego zalet i korzyści.

W sytuacji, kiedy osoba przeglądająca kod zgłasza uwagę, z którą nie zgadza się autor, każdy zespół powinien mieć prosty i szybki mechanizm podejmowania ostatecznej decyzji. Może to być szybkie pytanie do innego programisty/kierownika o zdanie lub zorganizowanie krótkiego spotkania w celu wspólnego podjęcia decyzji.

O autorze artykułu:

Mateusz Antkowiak | Programista PHP w Transparent Data

Zawodowy programista od prawie dekady. Swoje podróże z kodem rozpoczynał od takich egzotycznych kierunków jak Lazarus Pascal czy Delphi, a obecnie, ciągnie go w stabilne wody programowania obiektowego. Posiada ogromne doświadczenie w automatyzacji procesów, co przydaje się w jego obecnych RegTech taskach w Transparent Data |Prześwietl.pl.

--

--