Przegląd kodu, czyli kto ma rację?
Przegląd kodu to standardowa praktyka w pracy programisty, która ma na celu zapewnienie wyższej jakości tworzonego oprogramowania. Pozwala ona nie tylko wcześnie wykryć błędy i uprościć strukturę kodu, ale także ułatwia jego późniejsze testowanie oraz utrzymanie.
Choć temat na pierwszy rzut oka wydaje się oczywisty, perspektywa zmienia się, gdy wyjdziemy poza sam kod. Patrząc na code review od strony współpracy w zespole, a nie technicznej weryfikacji, można dostrzec, jak delikatny i pełen niuansów jest to proces.
Wystarczy wyobrazić sobie sytuację, gdy recenzent bezdyskusyjnie wymaga zmian. Jaka będzie twoja reakcja, gdy:
- narzuca zmiany nazw zmiennych lub funkcji, choć twoje są drobiazgowo przemyślane?
- narzuca rozbicie kodu na mniejsze funkcje, przez co logicznie powiązany fragment staje się poszatkowany i trudniejszy w nawigacji?
- wymusza zastosowanie wzorca projektowego, mimo, że prostsze rozwiązanie jest w zupełności wystarczające, a sens użycia wzorca jest mocno wątpliwy?
- dąży do mikrooptymalizacji, zdając sobie sprawę, że zysk wydajnościowy będzie znikomy?
Inny styl czy podejście do kodu nie powinno automatycznie oznaczać, że nasza praca nadaje się do kosza. Bez próby zrozumienia, łatwo o urazę. Pamiętajmy, że nawet w 100% trafne i świetnie uargumentowane uwagi, jeśli będą zbyt ostre lub podane w nadmiarze, mogą przytłoczyć drugą osobę i skutecznie ostudzić w niej zapał.
Czy jako recenzenci kodu powinniśmy mieć tę kwestię na uwadze?
Zdecydowanie tak, ponieważ w pracy zespołowej warto, a nawet trzeba zabiegać o relacje, a nie rację.
Powód jest prosty: rację ma się tylko na chwilę, a dobre relacje buduje się latami. W dyskusjach technicznych warto czasem odpuścić. Forsowanie swoich rozwiązań kosztem atmosfery w zespole to burzenie relacji cegiełka po cegiełce.
W praktyce sama merytoryka rzadko jest problemem – klucz tkwi w stylu prowadzenia dyskusji. Chodzi o to, by zamiast narzucać własne zdanie, dzielić się wiedzą. Dbając o partnerski i przyjazny ton od pierwszego do ostatniego komentarza, budujemy pozytywny dla obu stron dialog. Warto wyciągnąć pomocną dłoń: nie tylko wskazać błąd, ale też wyjaśnić jego źródło, aby następnie wspólnie wypracować optymalne rozwiązanie.
Dla przykładu, zamiast narzucać swoją rację, warto wyjść od sugestii:
Może warto X podzielić na dwie mniejsze funkcje?
Kluczem jest "Może warto", ponieważ tutaj zostawiamy jedynie propozycję i to od autora kodu zależy, czy z niej skorzysta.
Albo
Ta nazwa jest OK, choć trochę za długa. Co myślisz o jej skróceniu?
Tu ponownie wyrażamy swoje zdanie, ale nie narzucamy zmian. Ostateczną decyzję o nazwie zmiennej pozostawiamy autorowi kodu.
Albo
Co sądzisz o tym, by wydzielić tę część? Z tego co widzę, nie ma ona bezpośredniego związku z kodem biznesowym, więc jej wyodrębnienie mogłoby nam trochę ułatwić pracę.
I tym razem zostawiamy otwartą furtkę, tak aby autor kodu mógł zachować swoje pierwotne podejście.
Oczywiście można dojść do wniosku, że taki przegląd kodu niewiele wnosi, skoro w 99% przypadkach dopuszczamy możliwość implementacji rozwiązań, które nam, recenzentom, tak naprawdę nie do końca odpowiadają.
Nic bardziej mylnego, bo różne style pisania kodu nie muszą się wykluczać. Ostatecznie chodzi przecież o działające oprogramowanie, które spełnia wymagania biznesowe i jest odpowiedzią na potrzeby użytkowników.
Wyjątkiem jest ostatni 1% – sytuacje krytyczne, które mogą przekreślić szanse na sukces w projekcie. Tutaj nie ma dróg na skróty ani miejsca na kompromisy. W takich sytuacjach stanowcze weto recenzenta jest wręcz koniecznością, a przyjęcie uwag i ich wdrożenie przez autora jedyną słuszną postawą.
Dobre code review to nie lista żądań, ale partnerska dyskusja, nie czerwone kartki, ale trafne sugestie i pytania pomocnicze. Gdy gra się w jednej drużynie, chętniej przyjmuje się uwagi – również te krytyczne z najważniejszego 1%. Moim zdaniem umiejętność przeprowadzania wartościowego code review świadczy o dojrzałości programisty.