Siedzisz nad rozwiązaniem problemu kilka dni. Wpadasz na jego rozwiązanie. Projektujesz klasy i metody. Jesteś z siebie ogromnie dumny... i nagle dowiadujesz się, że źle nazwałeś zmienne, metody są nieintuicyjne, cała klasa do przepisania, a w ogóle to i tak nie do końca zrozumiałeś początkowy problem. Jak uniknąć zderzenia z brutalną rzeczywistością, oraz jak obiektywnie recenzować kod innych? Podpowie Ci to niniejszy artykuł.
Co to właściwie jest code review i po co? Review kodu odbywa się najczęściej po zaimplementowaniu danej funkcjonalności, wówczas pozostali członkowie zespołu decydują, czy dane zmiany wprowadzić do projektu, czy też wymagają jeszcze pewnych usprawnień. Uwagi i sugestie na code review można podzielić na 3 poziomy:
Poziom 1 – TYPOs, literówki i drobne uwagi
Jest to najbardziej podstawowy rodzaj uwag, najprostszy do zauważenia przez reviewerów.
Jak widać na powyższym przykładzie, metoda addd powinna zostać zmieniona na add. Szczegół nad szczegółami, jednak takie uwagi również są potrzebne, żeby utrzymać czysty kod. Sugestie znamy nazw są również potrzebne:
Metoda przekształca kolekcję imion – powinny one mieć co najmniej 3 i (akurat w tym konkretnym przypadku) co najwyżej 30 liter, następnie zamienia wszystkie litery na małe. Co tutaj można zmienić? Otóż nazwa metody, mimo że wyjaśniająca jej implementację, jest dość długa, można pomyśleć nad lepszą:
Nazwa metody została skrócona, co więcej, nie wyjawia ona na świat zewnętrzny szczegółów implementacyjnych – metoda „wie”, o jakiej długości mają być imiona – ktoś, kto tę metoda wywoła, nie musi znać tych detali. Zostały wyekstraktowane również zmienne lokalne opisujące liczby 3 oraz 30 – już nie są tzw. „magic numbers”. Uwagi tego stylu są jak najbardziej również potrzebne, jednak nie powinniśmy ograniczać się jedynie do nich.
Poziom 2 – Struktura klas i metod, testy, wzorce projektowe
Dodanie tego rodzaju uwagi wymaga już dłuższej i bardziej wnikliwej analizy. Weźmy pod uwagę jeszcze raz przykład z poprzedniego rodzaju uwag:
Uwagi z poziomu 2 mogą być następujące:
- Czy kolekcja names powinna być polem, czy może jednak powinna zostać podana jako argument metody?
- Czy kolekcja names powinna zostać listą, czy może jednak bardziej odpowiednią formą dla niej byłby set? Czy akceptujemy duplikaty?
- Czy metoda applicableNamesLowercased powinna pozostać publiczna? Być może wywołujemy ją jedynie wewnątrz danej klasy, bądź w podklasach?
- Czy do powyższej metody zostały napisane testy?
Wracając do początkowego przykładu z kalkulatorem:
- Czy klasa nie powinna być oznaczona jako final? W końcu raczej nie będziemy rozszerzać klasy kalkulatora.
- Czy metoda add nie powinna być statyczna? Jeśli cała klasa to klasa w stylu util?
- A może jednak metoda add powinna pozostać niestatyczna, gdyż statyczne metody są trudniejsze w testowaniu?
- Czy do powyższej metody zostały napisane testy?
Jak widzimy, Uwag kwalifikujących się do poziomu 2 jest o wiele więcej – można wręcz powiedzieć, że w pewnych niuansach, ile ludzi tyle opinii. Wszystkie jednak powinny mieć wspólny mianownik – dążenie do jak najbardziej czytelnego kodu.
Poziom 3 – Logika biznesowa
Żeby dodać uwagę tego rodzaju, trzeba dobrze znać projekt, w którym się pracuje, najlepiej całościowo, nie tylko kawałek funkcjonalności. Chociaż czasami wystarczy zdrowy rozsądek i rzetelne czytanie kodu. Spójrzmy na metodę, która się już nam pojawiła:
Minimalna długość została zmieniona! Programista, który nie do końca zrozumie początkowe założenia (minimum 3 litery), napisze funkcjonalność „działającą”, nawet przetestowaną i testy przechodzą... Jednak ów programista testy napisał, opierając się na fałszywym założeniu! Uwagi w stylu poziomu 3 są bardzo cenione przez innych programistów.
Poznaliśmy już różne rodzaje uwag w code review. Co jednak w przypadku odwiecznych wojen (taby vs spacje) lub różnych opinii dwóch programistów? Pamiętajmy o bardzo ważnym słowie – KONWENCJA. Uważasz, że Twój kolega źle nazwał klasę? Zwróć najpierw uwagę na to, czy nie postąpił według konwencji, czyli według przyjętych (przez zespół) zasad. Nawet „nie do końca czysty kod” w dużym projekcie, jeśli jest trzymany w konwencji, jest dużo bardziej czytelny niż „clean code”, gdzie każdy z modułów ma inne podejście.
Bądźmy również otwarci na merytoryczną krytykę od innych – zamiast „znowu się przyczepił”, pomyślmy o uwagach na code review jako o możliwości samorozwoju i zauważenia innych form widzenia. Każdy programista czuje satysfakcję ze swojego kodu, to naturalne i zdrowe (byłoby źle, gdyby nie było tej satysfakcji). Jednak nie przywiązujmy się zbyt emocjonalne do swojego rozwiązania. W drugą stronę, samemu również nie „wyżywajmy się” na kolegach z zespołu, wywyższając się gdy znajdziemy pewien błąd w ich kodzie. Pamiętajmy, że code review ma wspólny cel – dostarczanie kodu wysokiej jakości.
