Dlaczego robienie code review nie jest bez sensu?

W dzisiejszym wpisie zamierzam poruszyć kwestię sprawdzania poprawności kodu wg różnych kryteriów. Co może być brane pod uwagę jeśli chodzi o takie sprawdzenie?

Może to być wiele rzeczy. Lecz zanim do tego przejdziemy, to chciałbym wyjaśnić po co w ogóle sprawdzać poprawność kodu? W codziennej praktyce każdego programisty pracującego w jakiekolwiek poważnej firmie jest to bardzo ważna część obowiązków. Sprawdzenie poprawności kodu (tzw. code review - będę używał tego określenia w dalszej części) zapobiega tworzeniu kodu, który wprawdzie działa, ale jest źle napisany.

Formatowanie kodu

Przez "źle napisany" należy rozumieć kod, którego formatowanie jest niezgodne z wytycznymi projektu, przez co każdy inny programista może mieć problem z szybkim odczytaniem kodu i ewentualnym wprowadzeniu dalszych zmian w nim.
Źle sformatowany kod, nie jest najważniejszą rzeczą sprawdzaną. Sprawdzanie formatowania kodu można sobie ułatwić za pomocą takich narzędzi jak JSLint czy JSHint, a w dodatku istnieją narzędzia które automatycznie, za nas, poprawią formatowanie kodu wg wyznaczonych wytycznych.

Usuwanie nadmiarowości kodu

Często spotykam się z sytuacją, gdy podczas tworzenia nowej funkcjonalności powstaje za dużo kodu. Jest to spowodowane tym, że tworząc nową funkcjonalność każdy stara się najpierw napisać kod tak, aby funkcjonalność zaczęła działać. Następnie powinien nastąpić etap w którym kod powinien zostać odchudzony z nadmiarowości (zbędne zmienne, pozostawione console.log, brak cache'owania zmiennych) i zoptymalizowany.
Osoba robiąca code review, powinna na to zwrócić uwagę. Dzięki temu, w przyszłości nie będzie możliwa sytuacja w której ktoś może się bać usunąć jakąś zmienną lub nieużywany kawałek kodu, bo nie jest pewna że to czegoś nie zepsuje. Nadmiarowy kod należy usuwać zaraz po skończeniu prac nad funcjonalnością.

Optymalizacja kodu

Optymalizacja jest bardzo ważna. Tworząc kod JS dla przeglądarek jest to wręcz kluczowa rzecz. Dzięki optymalizacji kodu jesteśmy w stanie:

polepszyć stabilność aplikacji,
zwiększyć jej wydajność,
czasem zmniejszyć wagę końcową kodu.

Problem z pisaniem bardziej optymalnego kodu polega na tym, że aby pisać kod optymalnie należy zdobyć odpowiednie doświadczenie (a to nie jest możliwe do osiągnięcia w krótkim okresie czasu). Dlatego, pod kątem optymalizacji, kod powinien być sprawdzany przez bardziej (lub najbardziej) doświadczoną osobę w danym języku programowania.

Testy kodu

Testowanie kodu jest już praktycznie codziennością w dużej części firm. Dzięki temu, że testujemy kod (testy jednostkowe, manualne, integracyjne), jesteśmy w stanie znaleźć miejsca w kodzie, które mogą powodować jakiekolwiek problemy, np. źle działająca aplikacja, brak zabezpieczenia danych wejściowych, dziwne zachowanie aplikacji, itd. itp.

Na początku nie byłem zwolennikiem pisania testów jednostkowych dla kodu JS, bo to jest proces, który momentami może zajmować drugie tyle czasu który jest potrzebny na utworzenie funkcjonalności. Załóżmy, że potrzebujemy 5 dni na napisanie czegoś co działa, a potem żeby coś dobrze przetestować potrzeba drugie tyle czasu (szczególnie na początku, gdy się tego uczymy - potem, jest tylko trochę lepiej). Wraz z pisaniem testów kodu, nierzadko poprawiamy od razu istniejący kod, tak aby nie sprawiał problemów oraz aby był lepszy.

Wiele razy dobrze napisane testy uratowały mnie przed przypadkowym zepsuciem istniejących funkcjonalności, dlatego warto to robić. Bowiem, koszt wynajdowania błędów jest wtedy zdecydowanie niższy i cały proces zajmuje zdecydowanie mniej czasu.

"Make it work. Make it pass all requirements. Make it very good."

To jest powiedzenie, które stosuję w swojej codziennej pracy. Co ono oznacza?

Make it working - pierwszy etap tworzenia kodu. Zbuduj coś co działa, po prostu.
Make it passing all requirements - drugi etap, w który dążę do tego aby kod spełniał wszystkie wymagania. Między innymi te które wymieniłem powyżej.
Make it very good - ostatni etap, o który bardzo ciężko jest dbać, gdy gonią terminy. Tak napisany kod praktycznie nie trzeba zmieniać. To jest kod, do którego nikt nie ma zastrzeżeń, a sprawia że stanowi on doskonały przykład, który może być analizowany przez innych członków zespołu w celu poszukiwania inspiracji rozwiązania swojego problemu.

W praktyce, ostatni etap jest osiągany czasami, gdyż nie zawsze jest możliwość, aby nad kodem posiedzieć dłużej, gdy gonią terminy. Lecz mimo wszystko, każdy z nas powinien dążyć do tego, aby nasz kod osiągał ten stan możliwie jak najczęściej. Dzięki temu stajemy się lepszymi programistami.

Traktuj sprawdzany kod jak swój

Kolejna zasada, którą stosuję w swojej praktyce. Jeśli ktoś mnie poprosi o sprawdzenie kodu, to musi się liczyć z tym, że będę chciał, aby taki kawałek kodu był jak najlepszy. Dzięki takiemu podejściu wiemy, że jakość kodu nie spadnie jeśli będziemy od niego wymagać tyle samo co od kodu napisanego przez nas samych.

Co ważne, w tej sytuacji część osób czasem się zapędza i żąda, aby osoba napisała kod dokładnie tak jakby ona to napisała. To jest bardzo niebezpieczna sytuacja, bo jak się często okazuje kod napisany inaczej nie zawsze jest lepszy, zazwyczaj jego standard jest taki sam jak poprzednio. Tylko kod jest inaczej napisany. W tym momencie marnujemy czas to, aby daną funkcjonalność robić 2 razy.

Podsumowanie

To o czym piszę powyżej to kwintesencja tego czym jest code review. To jest sprawdzenie kodu tak, abyś Ty lub Twój zespół (którego jesteś częścią) mógł łatwo i przyjemnie pracować nad danym kodem i mieć możliwość dalszego jego rozwoju. Poza tym, każdy jest w stanie wtedy się nauczyć czegoś nowego.

Trzeba jednak pamiętać, aby nie traktować robienia code review jako odwalania pańszczyzny i nie skupiać się tylko na wynajdowaniu nieprawidłowości w standardach kodu czy w nadmiarowości kodu. Z każdego kawałka kodu można się nauczyć czegoś nowego, czasem jest to tylko przypomnienie o czymś co wcześniej wiedzieliśmy, ale zdążyliśmy zapomnieć.

Nie należy też traktować robienia code review, jako sposobu na zrobienie komuś na złość. To nie o to chodzi, a psuje tylko atmosferę w zespole.