Code review, KONSULTACJE i czystość kodu KLUCZEM do SUKCESU
Rzadko zdarza się, żeby programista pracował sam nad komercyjną aplikacją. Równie rzadko zachodzi sytuacja kiedy ten sam zespół, bez żadnych zmian personalnych, siedzi nad projektem od jego powstania do zamknięcia. Jak w takim przypadku zadbać o to, żeby wszelkie zmiany nie powodowały bólu głowy? O tym dzisiaj, czyli dlaczego regularne code review i wspólne konsultacje oraz dążenie za wszelką cenę do upraszczania i ujednolicania kodu spowodowały, że pewna zmiana w naszym zespole nie spowoduje absolutnie żadnych zmian w tempie i jakości pracy.
Temat code review i dbania o jakość kodu to nieodłączny element dyskusji programistów na konferencjach i przy kawie. Jednak tym razem skupię się na jednym zagadnieniu z tym związanym. Na zachowaniu ciągłości tempa rozwoju projektu w przypadku zmian w składzie zespołu.
Jak to odchodzisz?!
Spora część z osób pracujących kilka lat jako programiści spotkała się z tą sytuacją. Klepiecie sobie radość „bardzo ważne dla biznesu formatki” i nagle dostajecie informację, że wasz kolega odchodzi z pracy. Pracowaliście razem już dłuższy czas. Nikt nikomu nie wchodził w drogę. Każdy miał do siebie zaufanie. Dlatego ta informacja jest dla Ciebie nieprzyjemna z dwóch powodów.
Po pierwsze zżyłeś się z kolegą. Ale tego nie ominiemy.
Drugą sprawą zaś jest fakt, że skoro nie wchodziliście sobie w drogę i ufaliście wzajemnie to prawdopodobnie nie wiesz co dokładnie robił, prawda? Nie jesteś w stanie powiedzieć nad jakimi plikami czy klasami ostatnio pracował. Ba, ciężki Ci sobie przypomnieć jakie rozwiązania wprowadzał w kodzie. Sytuacja jest jeszcze gorsza jeśli oboje pracowaliście nad zupełnie różnymi częściami systemu (np. w naszym przypadku podzieliliśmy się na backend i frontend mimo, że oboje jesteśmy full stack developerami).
Im mniejszy zespół tym problem jest większy. Ponieważ w dużej grupie wiedza o kodzie jest bardziej rozłożona po ludziach i jest mała szansa, że nikt inny nie dotykał kodu, którym zajmował się kolega. Jednak jeśli jest to zespół dwu-, trzy-, cztero- osobowy to zaczynają się kłopoty.
Co to jest?!
Najgorsza wersja tego najgorszego scenariusza to sytuacja kiedy kolegi już nie ma i zaczynacie przeglądać jego kod, żeby móc przejąć jego obowiązki i nagle łapiecie się za głowię bo nie macie pojęcia co gdzie jest. Ufaliście mu, ale jednak w niektórych miejscach stosował swoje podejście do kodu, ulubione wzorce itd. Być może nawet kod w każdym miejscu jest jakościowo w porządku. Jednak ciężko jest Wam prześledzić wszystkie procesy i zakamarki. Wdrożenie się w ten fragment projektu zajmie w zależności od złożoności od kilku godzin do przynajmniej kilku dni. Prawdopodobnie też uznacie, że będziecie powoli przepisywać te kawałki dostosowując je do swoich standardów. A potem Wy odchodzicie, inny kolega dostaje Wasz kod…
Zróbmy krótkie spotkanie. Masz wolny ten tydzień?
Mniej pesymistyczny scenariusz jest wtedy kiedy przekazywanie wiedzy jednak następuje. Tzn. kolega jeszcze nie odszedł, ale są to już jego ostatnie dni. Rezerwujecie sobie czas na spotkanie aby dowiedzieć się co, gdzie, z czym. Pytacie czy dużo tego jest. Dostajecie odpowiedź, że góra dwa-trzy dni i wszystko będzie jasne. Dwa-trzy dni kiedy nie robicie swoich normalnych zadań (albo dużo dłużej jeżeli przekazywanie nie blokuje całkowicie normalnej pracy). Siadacie wspólnie do kodu i czujesz się jak na wykładzie. Wykładowca opowiada Ci o rzeczach, które widzisz pierwszy raz na oczy kiedy on sam uważa wszystko za oczywiste. Im bardziej skomplikowany system tym różnica w ilości pytań i odpowiedzi będzie większa. Na koniec się okaże, że poświęciliście kilka dni życia żeby dowiedzieć się, że taki kod istnieje i autor miał tam jakiś swój pomysł. Teraz trzeba by to jeszcze zapamiętać…
Z taką sytuacją spotkałem się w poprzednim projekcie. Co prawda zespół był większy więc zadziałało rozproszenie wiedzy wśród ludzi jednak mimo wszystko pojawiało się mnóstwo pytań „co autor miał na myśli”.
Szybka wymiana wiedzy
Jednak sytuacja może wyglądać zupełnie inaczej. Wystarczy jeden prosty trik.
Przez ostatnie pół roku pracowałem w dwuosobowym zespole nad aplikacją, która była częścią dużo większego systemu, z którym była zintegrowana (właściwie to tylko przepychała dane w jedną i drugą stronę, ale biznes mówi, że ma to sens). Podzieliliśmy się pracą w ten sposób, że ja wziąłem frontend bo chciałem poznać Angulara, a kolega zajął się backendem bo woli integracje niż CSSy. I w tym momencie zdarzyło się, że kolega, z którym pracuję przydałby się w innym otwieranym projekcie. A że nasz projekt właściwie będziemy niedługo przekazywać klientowi to transfer został zaklepany i ów kolega jest w trakcie migracji (powoli będzie coraz więcej czasu poświęcał drugiemu projektowi).
Na pierwszy rzut oka tragedia i w ogóle jak ja mogłem dopuścić do tego. Przecież skoro w taki, a nie inny sposób podzieliliśmy się pracą to w tym momencie projekt stanie, albo transfer wiedzy będzie trwał do końca maja albo dłużej.
Otóż nie. Transfer wiedzy trwał mniej więcej godzinę i w tym czasie zdążyliśmy jeszcze pośmieszkować o naszych projektach. Po tej godzinie właściwie mogę całkowicie przejąć jego zadania i pracować jak gdyby nic się nie stało (uwzględniając ilość możliwej do wykonania pracy w czasie oczywiście).
Jak to możliwe? Tak, że transfer wiedzy trwał tak naprawdę cały czas.
Rozmawiaj
Przede wszystkim mimo, że pracowaliśmy w zupełnie różnych obszarach to konsultowaliśmy ze sobą proponowane rozwiązania. Kiedy któryś z nas chciał coś zmienić to najpierw pytał o opinię i ewentualne inne propozycje. W ten sposób załatwiliśmy dwie sprawy – wiedzieliśmy nad czym pracuje druga osoba i wiedzieliśmy jak działa proces, który będzie dodawać albo zmieniać.
Rozmowa o wprowadzanych zmianach załatwia przede wszystkim warstwę wiedzy biznesowej. Bo żeby o czymś się wypowiedzieć to musisz wiedzieć jak to działa. Więc słyszysz co z czym i w jaki sposób ma być połączone.
Nie zawsze takie rozmowy były prowadzone przed zrobieniem czego. Czasami po prostu pokazywaliśmy i konsultowaliśmy dodany fragment kodu upewniając się, że nie powstał jakiś logiczny potworek. Wystarczy zadzwonić (jeśli zespół jest rozproszony) lub podejść i zagadać poświęcając maksymalnie kilka minut.
Sprawdzaj
Dzięki temu, że pracujemy jako full stack developerzy potrafimy się poruszać w obu obszarach, które zawiera projekt. Dlatego mimo, że zajmowałem się frontendem to każdy kawałek kodu backendowego przechodził przez moje ręce. Zanim coś trafiło do głównej gałęzi w Gicie to ja to widziałem, sprawdzałem i ewentualnie dopytywałem mając jakieś wątpliwości lub zgłaszałem uwagi. Dzięki temu żadne techniczne rozwiązanie, wzorzec, biblioteka itd. nie przeszły bez mojej wiedzy. Nie musiałem się mocno zagłębiać w każdą linijkę. Jednak fakt, że widziałem ten kod kilka razy w tygodniu powodował, że nie był on dla mnie czymś obcym. Działało to oczywiście też w drugą stronę, tzn. żaden mój kod nie pozostał schowany przed wszystkimi i przepchnięty po cichu.
Code review załatwia sprawę transferu wiedzy stricte technicznej związanej z samą strukturą kodu i plików. Oczywiście jeśli w zespole pracuje więcej osób to ta wiedza dostanie się po kawałku każdemu, ale mimo wszystko zostanie przekazana.
Jeśli ktoś mówi, że nie ma w zespole czasu na code review to powinien być potem pierwszą osobą, która będzie musiała tłumaczyć biznesowi skąd się wzięły opóźnienia i paraliż pracy jaki wtedy tymczasowo powstał w przypadku czyjegoś odejścia.
Pilnuj jakości
Dwa poprzednie punkty ekstremalnie mocno upraszczają oddanie komuś innemu Twojej części pracy w przypadku jakichkolwiek przetasowań. Jednak czymś co domyka ten proces i w największym stopniu wpływa na sam moment konieczności przekazania pracy jest dbanie o to co się robi. Przyjęliśmy prostą taktykę – jak najmniej jak najbardziej spójnego kodu. Jeśli coś da się zrobić prościej to prawie zawsze była decyzja, że tak to będzie zrobione. Do tego większość elementów systemu pisaliśmy w taki sam sposób.
Dzięki temu opowiadając o jednym module tak naprawdę opowiadam o wszystkich. Bo jeśli jest zachowana spójność to może i zmieniają się nazwy klas, ale nie zmienia się ich struktura. Flow jest takie samo i jeśli będę szukał konkretnej rzeczy to wystarczy mi wiedzieć jakiego obszaru dotyczy i od razu ją znajdę tam gdzie tego się spodziewam.
Dodatkowo mniej kodu napisanego to mniej kodu, o którym trzeba opowiadać. Taktyka usuwania wszystkiego co zbędne sprawdziła się u nas doskonale. Wszystkie części wspólne są powyciągane do jednego miejsca, a każdy kawałek to raptem kilka linijek kodu współpracujących ze sobą klas. Bez wymyślania koła na nowo i architektury rodem ze skostniałych korporacji. Łatwiej jest przekazać całą wiedzę jeśli trzeba to zrobić dla trzech klas z kilkoma metodami, które się w przejrzysty sposób wywołują niż dla 15 warstwowej architektury enterprise, która posiada 24 mappery, przepychające dane logowania użytkownika z API do bazy.
Równie ważne jest dbanie o takie rzeczy jak nazewnictwo. Jeżeli ktoś chce sprawdzić jak działają płatności to niech ma możliwość znalezienia czegoś co się nazywa np. Payments. Każde pytanie „gdzie znajdę kod do…” powinno wywoływać u Ciebie moment zastanowienia czy dobrze przemyślałeś nazwy plików i folderów.
Małe kroczki
Musisz zakładać, że w pewnym momencie zespół będzie przechodził zmiany. Jest to nieuniknione jeśli projekt ma trwać kilka lat. Dlatego jeżeli nie chcesz zostać z bólem głowy i paniką w oczach to zadbaj o przekazywanie wiedzy małymi kroczkami. Commit po commicie. Na co dzień. Rozmawiajcie. Przeglądajcie wzajemnie kod. I dbajcie o to, żeby Wasze linijki był na tyle proste, że będzie się dało o nich opowiedzieć przy okazji przerwy na kawę.
Najgorsze co może być, zwłaszcza w bardzo małych zespołach, to zostawienie osób samym sobie. Nawet jeżeli każdy z Was pisze dobry kod i wie jakie są ogólnie przyjęte standardy to jednak wiedza o tym co, gdzie i jak dokładnie zostało zrobione wymaga czasu i śledzenia zmian na bieżąco. Zostawiając transfer wiedzy na moment zaraz przed odejściem kogoś masz gwarancję, że zostaniesz z setką pytań do osoby, której już nie ma i z kodem, którego nie znasz. Nie warto.
Uważam, że rekruter powinien powiedzieć jak ma być wykonany kod lub jak go poprawić, by był akceptowalny przez firmę. Każdy manager ma inne podejście do jakości kodu, a zgadywanie czy kod spodoba się rekruterowi jest po prostu słabe, nie mniej powszechne w tej branży. Gdybym się zmienił z kierownikiem miejscami, też zapewne znalazłbym błędy w jego kodzie, praktykach itp…
Dwa przykładowe sposoby radzenia sobie z rotacją kadr, i jeden gratis
Problem częściowo się rozwiązuje jeżeli stosujemy praktyki opisane w DDD (niebieska książka)
Jeżeli kod odzwierciedla domenę biznesową a zespół pracuje nad ubiquitous language to sam styl kodowania nie będzie problemem
2. Wyznaczanie osób odpowiedzialnych za części składowe systemu + zastępcy.
Przeważnie największa rotacja panuje na niższych stanowiskach, osoby odpowiedzialne nie muszą pisać nawet takiego komponentu, ale muszą go reviewować a także pilnować produktyzacji.
Osoba odpowiedzialna powinna tworzyć/ utrzymywac minimalną dokumentacje high-level design, usage/test, readme i pilnować aby ktoś od czasu do czasu próbował taki komponent postawić korzystając tylko z dokumentacji.
3(gratis). można i tak: https://www.thelayoff.com/t/Pz7cuWI
@ArFeN że niby rekruter powinien dyktować styl kodu? to jakiś bezsensowny pomysł rodem z Bangalore