Przeglądy kodu

Kilka słów o przeprowadzaniu code review. A zatem po co? Czy nie wystarczy podłączyć Sonara czy coś w tym stylu. Z pomocą przyszedł mi cytat zamieszczony na strone 33rd Degree Conference: A single conversation with a wise man is better than ten years of study. I właśnie dlatego warto przeprowadzać code review :) Prowadząc taką rozmowę z f2f robimy użytek efektu human infection, czyli przekazujemy sobie wiedzę i ideę w bardziej naturalny sposób niż na papierku, albo ciągiem bitów.

Informacje początkowe

Dalej streszczam mój sposób na przeprowadzanie przeglądu kodu, ale najpierw parę tematów wprowadzających.

Jak często przeprowadzać przeglądy?
Tu nic odkrywczego nie wymyślę, jeśli napiszę, że: raz na wydanie albo raz na iterację albo w regularnych odstępach czasu albo po zamknięciu zadania albo na żądanie :)

Kto przeprowadza przegląd?
Jestem fanem ról, a sceptykiem stanowisk więc sądzę, że przegląd powinien przeprowadzać lider. W żadnym razie lidero-kierownik mający pod sobą kilkanaście osób. Mówię o liderach np. konkretnych komponentów systemu, którzy przewodzą maksymalnie pięciu osobom. Jeśli brak takiego liderowato-centrycznego uporządkowania, to przegląd robią ludzie, którzy mają mandat od zespołu wskazujący, że "ten zna się na rzeczy". Rzecz jasna poszczególni programiści z biegiem czasu nabywają kompetencji do przeprowadzania przeglądów. Zespół żyje.

Jak szczegółowy ma być przegląd?
Ufam, że ludzie są mądrzy. Wystarczy pokazać im co i dlaczego robią źle, aby potem zgeneralizowali ideę i wprowadzali poprawki w reszcie kodu. Nie ma potrzeby pokazywania palcem absolutnie wszystkiego. Wiem, wiem pewnie, że są wyjątki. Ale co do zasady, wspomniane założenie ufności jest dość użyteczne.

Ile czasu zajmie przegląd?
Jeśli znasz i rozumiesz kod to w okolicach 30 min plus drugie tyle na rozmowę. Jeśli nie znasz kodu (bo na przykład analizujesz kod klienta) to już jest zupełnie inna bajka. Megaistotne jest zebranie informacji co z czym się je, żeby nie wydzwaniać do ludzi co parę minut. Sam późniejszy przegląd to 8h wzwyż.

Przegląd krok po kroku

1: Zdefiniuj w IDE tasktagi //SMELL i //REFACTOR
Będą potrzebne do oznaczania w kodzie różnych ciekawostek.

2: Ogranicz się do kodu: jednego user story | jednego zadania | jednej funkcjonalności
Generalnie mogę wpaść w cały moduł i nie wyjść z niego przez tydzień. Ale zgodnie z zasadą zaufania, chcemy wyłapać symptomatyczne niedociągnięcia delikwenta, a nie prześwietlić jego kod od A do Z. Który zatem kawałek kodu wybrać? Tu przyda się rozbudowany instynkt Dr Housa. Trzeba z premedytacją wybrać taki fragment kodu, który potencjalnie mógł programiście sprawić trudność. Przykład: ostatnim razem ustaliliście nad czym ma popracować, wiec teraz sprawdź podobne zadanie albo kątem ucha usłyszałeś jak rozmawia o jakimś trudnym zadaniu - sprawdź właśnie to. Jasne, że nie robimy tego z czystej złośliwości, lecz właśnie w tego typu zadaniach jest okazja, aby wyłapać największe niedociągnięcia i najszybciej skorygować styl pisania.

3: Wygeneruj diagram zależności między klasami/pakietami
Najczęściej korzystam z wbudowanych widoków Eclipse lub ficzerów ReSharpera, wspomagając się kartką. Teraz zaczynam zapoznawać się z CDA i jeszcze nie mam wniosków. Jeśli chodzi o C++ to Visual Assist X jest jakimś tam ułatwieniem. Ale konwencje utrzymywania projektów i plików w C++ są tak zróżnicowane, że niemal za każdym razem wymaga ode mnie kreatywnego podejścia. Zazwyczaj kończy się na Eclipse C/C++ oraz kartce i długopisie. (Nie, nie rysuję w tym przypadku diagramów w narzędziu UML, bo za długo to trwa; ewentualnie potem, gdy okazuje się na przykład, że dziesięciopoziomowa hierarchia dziedziczenia sprawia kłopoty, to coś tam przerysuję żeby efekciarsko wyglądało w raporcie).

4: Pobieżnie przyjrzyj się każdej klasie w wybranym fragmencie kodu
Przyglądamy się tu pod następującymi kątami:

• ogólne wrażenie - elegancja kodu, czy woła o pomstę do nieba?
• nazewnictwa - czy nazwa oddaje intencję?
• wielkości klas i metod
• Ilość współpracowników

5: Prześledź podstawową obsługę żądania
Chodzi o prześledzenie ścieżki od momentu, w którym ktoś zainicjował przetwarzanie

do chwili, gdy system odpowiedział na nie.

Oznacz //SMELL miejsca, w których kod wydaje Ci się niewłaściwy. Przez //REFACTOR oznacz miejsca, w których od razu wiesz, co należy zrefaktoryzować.

6: Przygotuj podsumowanie
Byle krótkie w punktach, które uwzględni relację kodu do: obowiązującego standardu kodowania, czystości kodu, przyjętych zasad architektury.

7: Przekaż informacje zwrotną
w rozmowie f2f oraz wskazuj kierunki poprawy, ewentualnie zaimprowizuj mikorszkolenie. Zanotuj sobie, aby zweryfikować, czy postanowienia z rozmowy zostały wdrożone. Jeśli ludzie będą zapominać, przeglądy nic nie wniosą i wszystkim odechce się w nie angażować.

Czy należy wnioskować, że nienawidzę narzędzi?

Absolutnie nie. Takie super gadżet jak Sonara przeanalizuje za Ciebie masę kodu i wskaże miejsca, w które warto zajrzeć osobiście. Jestem wielkim fantem użytecznych narzędzi z zastrzeżeniem, że nie wolno nam zafiksować, że narzędzia zrobią wszystko, a my będziemy mogli zwolnić się z myślenia.

Dodatkowo jestem entuzjastą minimalizowania ilości otwartych zadań (work in progress). Jeśli robię przegląd i używam fajnych okienek, wpisuję komentarze (które potem trzymane są w repo), to te komentarze tam sobie będą i będą, aż może ktoś się nimi zajmie, rośnie tendencja do odkładania. Jeśli teraz robię przegląd i mam kilka słów podsumowania na kartce, to będę dążył to tego, aby jak najszybciej złapać daną osobę, przekazać co mam do przekazania i niech ona to już zrobi. Problem zdiagnozowany, rozwiązany, można pisać dalej.