Code Review

lup

Code review to jeden z najlepszych sposobów, aby pomóc zespołowi upewnić się, że pisze najlepszy możliwy kod. Daje możliwość nie tylko wyłapania błędów, ale również przedyskutować wprowadzane zmiany, dając wszystkim członkom zespołu możliwość zapoznania się z nowymi funkcjonalnościami.

Z jednej strony code review wydawać się może zbędnym wysiłkiem w sytuacji gdy aplikacja jest pokryta testami TDD. W tej sytuacji możemy bez większych przeszkód zmieniać kod lub dopisać jego nowe elementy wraz z kolejnymi testami, które obrazują jak oczekujemy, aby kod działał. Uruchamiamy testy i w jednej chwili wiemy czy aplikacja działa lub nie.

Z drugiej strony jako autor kodu zawsze stawiam pewne założenia dotyczące sposobu działania systemu i czego użytkownik potrzebuje, dlatego przegląd kodu przez drugiego programistę daje szanse wyłapania błędów logicznych lub problemów strukturalnych, których komputer nie może zobaczyć. Dodatkowo osoba wykonująca code review zaznajamia się z fragmentem systemu, nad którym może aktualnie nie pracować i w sytuacji kryzysowej dokonać niezbędnych poprawek lub zmian.

W niniejszym artykule chciałbym podzielić się paroma praktykami, w którym code review przebiega płynnie niezależnie po której stronie procesu oceny stoimy.

1. Nie karz. Celem code review jest powstanie najlepszego możliwego kodu. Staramy się być pomocni i upewniajmy się, że komentarze nie wydają się oskarżycielskie.

2. Zadawaj pytania. Należy zadawać pytania odnośnie wszystkiego, czego nie mamy pewności. Kod jest jasny dla autora, niemniej dla recenzenta duże partie kodu nie zawsze bywają oczywiste. Dzięki pytaniom i odpowiedziom uzyskujemy większą klarowność nad daną funkcjonalnością. W zespole pracujemy razem. Kompromis wymaga rozmowy i zrozumienia.

3. Nikt nie jest właścicielem kodu. Staramy się unikać sytuacji „własności kodu”. Gdy któryś z członków zespołu przypisuje własność kodu, wówczas staje się mniej podatny na sugestie i zmiany sądząc, że pierwotna wersja jest doskonała.

4. Przekaż pełną recenzję. W trakcie code review przeglądamy cały kod, a nie skupiamy się wyłącznie na fragmentach lub klasach. Jeśli czegoś nie wiemy… patrz pkt. 2. W przeciwnym wypadku narażamy autora na wykonywanie bezproduktywnych poprawek tylko dlatego, że recenzent nie przejrzał całego kodu.

Reasumując, staramy się nie tylko patrzeć na składnie kodu, ale także na jego architekturę. Przekazujemy własne opinie i sugestie dochodząc wspólnie do jak najlepszych rozwiązań.

W internecie oczywiście jest wiele tipów i praktyk code review. Najlepiej znanym jest artykuł na gitlabie: https://gitlab.com/help/development/code_review.md niemniej mam nadzieje, że i nasze drobne sugestie przekonają czytelnika o pozytywnych aspektach niniejszego procesu.

Autorem tekstu jest Marek Rode.

Zobacz również artykuły o podobnej tematyce

B2E, czyli skuteczne wsparcie organizacji pracy
B2E banner

Jak sprawić, by organizacja efektywnie funkcjonowała na współczesnym rynku? Każdy kto prowadzi firmę wie, że kwestia ta potrafi spędzać sen...

Aplikacja mobilna brakującym ogniwem omnichannel?
aplikacja mobilna

W 2018 r. strategia omnichannel wydaje się standardem. Wszystkie liczące się marki posiadające sieć sklepów stacjonarnych połączyły w większym lub...

Czy człowiek może być kolorem? Kapitał ludzki, jako największa wartość w organizacji
ludzie-1

Chcąc prowadzić innowacyjną i ponadczasową firmę należy bacznie obserwować indywidualne predyspozycje pracowników. Nie każdy z nich nadaje się do pracy...

Zobacz więcej wpisów
  • http://oferownik.pl Robert Marketingowiec

    Nie da się być mocnym we wszystkich kanałach komunikacji, trzeba sobie wypracować te które najbardziej się wybijają i się ich trzymać.

  • Marcin

    „Dawno temu skupialiśmy się na ważnych rzeczach, czyli rozwiązywaniu problemów i logice biznesowej. Dzisiaj jesteśmy prawie całkowicie skoncentrowani na abstrakcji i generalizacji.” – i to wszystko wina frameworków? Czy to wina cegieł i zaprawy że murarze krzywo wybudowali mur, czy jednak problemem było nieprofesjonalne podejście fachowców, którzy ubzdryngoleni przyszli do pracy? Nie możemy zrzucać winy na narzędzia, za to że źle tworzymy aplikacje. Powinniśmy raczej starać się dostrzec fakt, że problem leży w nas samych i spróbować go zrozumieć… oraz coś z nim zrobić. I na pewno nie jest metodą na ów problem podejście, że „powinniśmy używać PHP, a nie architektury do budowania aplikacji” – taki tok rozumowania jest idealnym dowodem na to jak wiele osób nie rozumie czym jest architektura. Architektura to nie framework! Posłużę się kolejnym odwołaniem do budownictwa: czy patrząc na kościół pierwsze co widzimy to to, z czego jest on zbudowany czy raczej jakie jest jego przeznaczenie. Albo: czy widząc bramę pierwszą naszą myślą jest „użyli cegieł z Castoramy i wkrętarki boscha” czy raczej „to jest brama – służy do tego by gdzieś wejść lub skądś wyjść”?
    To prawda – we współczesnym programowaniu zapominamy o logice biznesowej, ale nie jest to wina architektury, tylko raczej braku dyscypliny w nas samych – nie interesuje nas to, nas jara głównie fakt że piszemy np. w kotlinie bo to jest teraz modne. „Zrobię to na symfony, zapytania puszczę doctrinem a obiekty będę serializować jms’em” – to nie jest opis architektury, to lista zakupów – jakich narzędzi potrzebuję by zrealizować cel. Szkoda tylko, że przez to cel ląduje na drugim (albo i nawet dziesiątym) miejscu – a celem jest np. zbudowanie aplikacji do przechowywania dokumentów. Ale to nie decyzja twórców bibliotek/frameworków, że cel staje się dla nas mało istotny. To nie Fabien Potencier każe nam całą logikę wrzucać do akcji w kontrolerze, to nie Jonathan Wage podpowiada nam, by nasze biznesowe obiekty bezpośrednio wywoływały doctrine’a! To my, my programiści sami doprowadzamy do tego, że nasze aplikacje przypominają niestrawione resztki pokarmu które codziennie zostawiamy w porcelanowym ekspresie. I póki nie nauczymy się odseparowywać logiki biznesowej od narzędzi to dalej będziemy tworzyć legacy code.
    Co do argumentu, że niektóre z zależności mogą zostać po kilku latach porzucone i nie będą dalej rozwijane… serio? I proponujesz phpti jako alternatywę do twiga albo smarty? Wszystko może zostać po kilku latach porzucone, to do obowiązków programisty należy m.in. nie pozwolić na to, by moduły/komponenty nie były sprzężone z innymi elementami czy zewnętrznymi bibliotekami.
    A co do bogactwa wiedzy i tego co się dzieje pod maską – ogólnie nie powinno się używać narzędzi, których się nie rozumie: jak działają, dlaczego tak działają itp. By potem nie było sytuacji w stylu senior piszący od 5 lat w laravelu nie potrafi odpowiedzieć na rozmowie kwalifikacyjnej na pytanie „Co robi funkcja die()?”…

    TL;DR: nie zgadzam się 😉