Chciałbym poruszyć temat praktyki code review, w tłumaczeniu na język polski przeglądu kodu. Z jednej strony, praktyka ta jest dobrze znana, obowiązkowa i rozpowszechniona w różnych środowiskach agile. Chciałbym natomiast skupić się na kwestii pożyteczności przeglądu i różnych sposobach ulepszenia.

Istnieje kilka etapów rozwoju zespołu i w każdym z nich może być zastosowana inna forma przeglądu kodu. Niektóre zespoły znają ten proceder tylko z mądrych książek, lecz w innych, bardziej oświeconych zespołach przegląd kodu jest obowiązkową procedurą poprzedzającą wprowadzenie kodu - bez przeprowadzenia code review przyjdzie duży, zły, brodaty architekt i wymiecie wszystko z powolnego kodera :).

Nawet jeśli przeprowadzi się przegląd kodu, i tak nie ma gwarancji na osiągnięcie korzyści tak żywo opisywanych w mądrych książkach przez guru elastyczności. Na czym więc polega problem?



Po pierwsze, przegląd kodu bardzo często przybiera formę kontrolowania stylu (style check). Jest to najprostsza forma stosowania komentarzy, która może być użyta automatycznie, szczególnie jeśli styl nie jest narzucony przez żadne narzędzia. W wypadku, gdy narzędzia inspekcji nie mają żadnych kategorii, nawet inni programiści nie będą w stanie znaleźć miejsca dla rozsądnych założeń w morzu komentarzy.

Po drugie, kontrolowaniu stylu towarzyszy syndrom "Nie umieściłbym tego w ten sposób". Zawsze istnieją różnice w terminach podczas tworzenia kodu, zależne od samego programisty. To samo tyczy się różnych podejść w rozwiązaniu tego samego zadania. Bardzo ważne jest, aby zrozumieć, w którą stronę zmierza obecne rozwiązanie i czy jest ono lepsze/gorsze od oferowanego, ponieważ jeśli nikt nie zmieni obecnego rozwiązania, może to doprowadzić do wielu późniejszych zmagań i prawdopodobnie negatywnych skutków.

Po trzecie, sprawdzającym może po prostu brakować kontekstu dla pełnowartościowego przeglądu. Jeśli główny pattern rozrasta się w zespole, podczas gdy odrębny kod jest w posiadaniu jednej osoby, reszcie zespołu będzie po prostu brakowało kontekstu, który umożliwiłby dostarczenie cennych wskazówek. Jeśli mimo wszystko komentarze zostaną zapewnione, autor będzie mógł użyć swojego doświadczenia, aby zakomunikować, że wszystko jest w porządku, a sprawdzający po prostu wychwyci błędy.

Po czwarte, właściwie już jest za późno. Problem z każdą inspekcją polega na tym, że jest ona przeprowadzana już po wykonaniu zadania, t.j. główne działania programistyczne zostały podjęte, główny moment już minął, inne zaległości czekają na wykonanie i istnieje silne pragnienie, by sprawdzić kod. Dlatego sprawdzający niechętnie zajmują się dzieleniem klas na prostsze i mniejsze, aby przypisać zobowiązania do metod i poprawić jakość testów i innego kodu.

Także, inspekcja kodu może wywołać patologiczne przypadki niekończących się iteracji. Jeśli jeden ze sprawdzających chce, aby rozwiązanie wyglądało inaczej, może rozpocząć szereg "iteracji", w których autor będzie się starał dostosować swoją decyzję do decyzji sprawdzającego (może to być punkt widzenia głównego programisty lub architekta), lecz prawdopodobne jest, że poniesie porażkę ze względu na fakt, iż jego mózg jest ulokowany na zewnątrz mózgu sprawdzającego. Wynik końcowy jest długotrwały i nieefektywny, zarówno dla autora jak i dla komentującego.

Jak powinien wyglądać przegląd kodu, aby był bardziej wydajny?

Aby przegląd kodu był wydajny, wszyscy uczestnicy powinni go chcieć i traktować go jako narzędzie, a nie jako koniec sam w sobie. Poniżej podanych jest kilka rad mających na celu ulepszenie tego procesu.

Po pierwsze, podczas trudnych zadań wskazana jest praca w parach, przynajmniej w niektórych momentach lub w kluczowej części zadania, np. projektowania. Pociągnie to za sobą "wspólny" kod/rozwiązanie, co uprości przegląd na kilka sposobów: autor nie będzie przyjmował krytyki osobiście, jako że odpowiedzialność będzie podzielona. Podejmowane decyzje będą miały wyższą jakość, ponieważ praca nad nimi wymagała zaangażowania dwóch par oczu i dwóch mózgów. Dodatkowo, dzielny drugi pilot będzie w stanie zapewnić backup podczas przeglądu i zabezpieczyć autorską ideę poprzez jasne jej sformułowanie i zapewnienie, że idea nie była przypadkowa, lecz starannie przemyślana.

Po drugie, w trudnych przypadkach i bez obecności partnera rozsądnie jest przeprowadzać przegląd kodu dopiero po rozwiązaniu niektórych zadań. Umożliwi to ludziom zminimalizowanie ryzyka rozwiązania nieprawidłowo postawionych zadań lub zaoferowania złego rozwiązania dla prawidłowego zadania. I znów, dostęp do kontekstu po rozwiązaniu projektowym pozwoli na udzielenie trafnych komentarzy podczas przeglądu kodu, ponieważ wszyscy sprawdzający będą świadomi problemu i, chociaż częściowo, będą znać rozwiązanie.

Po trzecie, rozsądnie jest zautomatyzować kwestię stylu, aby nie wystąpiła podczas przeglądu. Wyeliminuje to pokusę dodawania komentarzy nie na temat, dzieki czemu uda się uniknąć punktu zapalnego i podtrzymać dialog w komentarzach.

Dodatkowo, powinno się ustalić formalne i nieformalne ograniczenia i zasady gry. Na przykład, różne narzędzia pozwalają na wyrażenie czyichś działań poprzez typ komentarza i status. Jeśli sprawdzający będzie ciągle zamykał swoje komentarze statusem "Nie do rozwiązania", będzie to wyglądało dość agresywnie. Tym samym, jeśli narzędzie przeglądu pozwala na "zawetowanie" przeglądu, może to znacząco popsuć relacje. Zamiast tego, lepiej po prostu napisać/zadzwonić/spotkać się z osobą i przedyskutować te sprawy osobiście. Komunikacja werbalna jest zazwyczaj dobrym sposobem na rozwiązanie kwestii niekończących się komentarzy.

I, na koniec, ważne jest zrozumienie celu całego procesu. Celem przeglądu nie jest demonstracja wyższości architekta nad kodującymi, ale uczynienie świata :) i zespołu lepszym. Przeglądy pozwalają rozpowszechnić wiedzę, wzorce i podejścia do różnych postawionych zadań. Ta wymiana doświadczeń zapobiega rozpowszechnianiu się nieskutecznych rozwiązań, ponieważ pozostali członkowie zespołu wiedzą, jakich problemów można uniknąć, jeśli podejmie się odpowiednie rozwiązanie.

Ogólnie rzecz biorąc, jak w przypadku każdego innego narzędzia, przegląd kodu będzie pożyteczny, jeśli przeprowadza się go zgodnie z zaplanowanym celem i w zespole z odpowiednimi kwalifikacjami. Oznacza to, że w przypadku trudnych zadań trzeba pracować w parach, nawet przez krótki czas, przed przeglądem. Jeśli nie uda się tego wprowadzić, trzeba przeprowadzić przegląd projektowania lub przynajmniej opisać swoje wyobrażenia i założenia podczas publikacji przeglądu. Ważne jest, aby zapobiec zepsuciu w zespole i uniknąć agresywnych komentarzy, bo nic dobrego z nich nie wyniknie.

Sergey Teplyakov
Expert in .Net, C++ and Application Architecture