sobota 26. dubna 2014

Dopřejte vašemu kódu pořádnou revizi

Revize kódu (code review) patří mezi techniky, které mohou posunout vnitřní a vnější kvalitu vašeho systému do vyšší ligy. Musí se však dělat správně a efektivně. Revidování kódu může být časově náročné a od manažerů víme, že čas jsou peníze. Proto se systematické revidování používá především u projektů, kde je kvalita nadřazená brzkému dodání. Na druhou stranu, pokud jste někdy dělali na projektu, který se dostal do spirály smrti díky tragické kvalitě, jistě se shodneme na tom, že pojem “rychlý vývoj” je vždy relativní. Dobré revize v konečném důsledku čas a náklady na projekt snižují.

Revidovat kód lze mnoha způsoby. Ti co programují v páru, provádějí revizi kódu nepřetržitě již během jeho psaní. V ostatních případech jsou fáze psaní kódu a fáze revidování v čase odděleny. Čím více je revize opožděna za implementací, tím jsou ale případné nápravy komplikovanější. Revize se také liší stupněm formálnosti, rozsahem (např. pouze kritické části aplikace nebo všechno), počtem revidujících (jeden nebo celá skupina) a tím, zda-li je autor kódu přítomen nebo ne.

Hlavní přínosy

K čemu konkrétně jsou vlastně revize kódu dobré? Pomáhají nám v především v těchto záležitostech:

  • Ověřování splnění zadání.
  • Objevování chyb a případných rizik.
  • Zlepšování kvality návrhu.
  • Zlepšování čitelnosti kódu.
  • Zlepšování udržovatelnosti kódu.
  • Šíření znalostí a způsobu řešení problémů.
  • Zvyšování zastupitelnosti členů týmu.
  • Autor kódu dostává zpětnou vazbu ke svojí práci a díky ní se rychleji zlepšuje.

Dobře revidovat kód je dovednost, kterou musíme neustále procvičovat a zlepšovat. Abychom zbytečně nemrhali časem, ale aby nám revize opravdu sloužily, musíme si pohlídat několik základních věcí. Sem s nimi ...

Co mám vlastně revidovat?

Co opravdu, ale opravdu nemáme jako programátoři rádi, jsou špatně zadané úkoly. Při implementaci už není prostor na neurčitost. Ta nás brzdí a blokuje. Psát jednoznačné a srozumitelné zadání je ctností dobrého analytika. “Chytře” zadaný úkol (mrkněte na SMART) zvyšuje pravděpodobnost správného pochopení a odpovídající implementace a efektivní revize. Těžko ověříme splnění zadání, pokud je odbyté nebo nejasné. I jako revidující si vyžádejme upřesnění zadání, pokud je to potřeba. Revizí přebíráme stejnou zodpovědnost za úkol jako ten, kdo jej naimplementoval.

Velký nebo malý úkol

Revize malé změny - hodně připomínek, revize rozsáhlé změny - vypadá to dobře.

Pokud chceme mít revize opravdu smysluplné, dekomponujme problémy na pokud možno co nejmenší úkoly a ty revidujme odděleně. Implementace úkolu by neměla trvat více než jen několik hodin. Příliš rozsáhlé změny ukryjí defekty kódu, špatný design, bugy a jiné nedostatky. Snaha pochopit cizí kód (někdy i svůj) je vysoce náročná na soustředění a to jak víme není zadarmo. Po krátké době jsme unavení tak, že přepínáme na autopilota a pravděpodobnost přehlédnutí problému letí nahoru.

Dohledatelný rozsah změny

Rozsah revidované změny musí být jasně dohledatelný. Proto je vhodné, aby každý commit do repository byl logicky mapovaný právě na jeden úkol. Buildovací servery pak z takových commitů umí vyrobit ke každému úkolu přehledné reporty se zvýrazněním změn v jednotlivých souborech. Bez kvalitních podpůrných nástrojů a jejich vzájemné integrace se revize snadno změní ve zmatené prohlížení potenciálně změněných souborů.

Dobrá kondice revizora

Když se pořádně vyspíme, není pátek odpoledne a nemáme kofeinový deficit, pak máme nakročeno ke kvalitní revizi. Všechno je najednou mnohem jasnější. Prostě nemá smysl revidovat, když je člověk unavený. Je to podobné jako s programováním, jenom s tím rozdílem, že sice žádnou chybu nepřiděláme, ale ani žádnou neobjevíme.

Potřebná úroveň znalostí

Schopnosti a dovednosti revidujícího by neměly být výrazně nižší než schopnosti autora kódu. Mohlo by se pak totiž stát, že se mu zvolené řešení i kód bez výhrad líbí, přestože ve skutečnosti trpí mnoha nedostatky. A taková revize nemá žádný význam. Revidovat by proto měli ti zkušenější z týmu. Revidující by měl mít dobré znalosti použitých programovacích jazyků, frameworků, návrhových vzorů a principů, a mnoha dalších věcí. Jedním z přínosů revizí je právě to, že méně zkušení členové týmu získávají skrze zpětnou vazbu z revizí mnohem rychleji nové zkušenosti a tím se profesně zlepšují.

Automatizujme

Používejme nástroje pro statickou a dynamickou analýzu kódu, které se budou automatizovaně spouštět v rámci buildů hlídajících kvalitu. Co lze zkontrolovat automatizovaně to nemusíme revidovat. Šetřeme čas a soustřeďme se na záležitosti, které za nás programy nevyřeší.

Optimalizujme workflow

Pokud zatím revize nepoužíváte, zavádějte je opatrně. Hledejte ideální workflow, které bude kompatibilní s vašimi vývojovými procesy. Vyzkoušejte více variant a vyberte tu pravou. Až se vám revize dostanou pod kůži v rámci menšího týmu, teprve potom je zavádějte ve větším rozsahu. Revize kódu mají své opodstatnění jak v agilních, tak i především ve formálnějších metodikách. A není problém kombinovat revize více typů. Například my v rámci našeho projektu děláme některé úkoly v páru (okamžitá revize). Většinu úkolů ale revidujeme ad-post bez autora nebo při komplikovanějších věcech i s jeho přítomností. Velké implementační celky ještě revidujeme více formální revizí se softwarovým architektem zákazníka.

Sociální aspekty revize

S revizí kódu je to jako se ženou - ani nevíte jak a už jste v problémech. Je potřeba k ní přistupovat opatrně, lehce našlapovat a ve vyjadřování volit slovník gentlemana. Výstupem revizí by měly být připomínky, které zlepší kvalitu kódu a nezhorší vztahy v týmu. Případná kritika by měla být věcná, slušná a cílená na konkrétní problém. Žádné osobní útoky. Svojí nešikovností nebo dokonce špatným úmyslem, můžeme rozpoutat nepříjemné konflikty, které se projeví na fungování týmu. Velkým problémem je také nadřazený přístup zkušenějších vývojářů k těm méně zkušeným. Zdravě fungující tým by si měl s podobnými problémy poradit, ale i přesto buďte opatrní.

Motivace odvádět kvalitní práci

Dobře motivovaný tým chápe revize jako další prostředek ke zlepšení výsledků svojí práce. Na týmy s problematickou úrovní práce může mít zavedení revizí také velmi příznivý efekt. Programátoři, kteří kvalitu kódu do té doby příliš neřešili a v tichosti si něco bastlili, mají najednou nové starosti. Jejich kód začne někdo jiný číst a dokonce připomínkovat. Někdy i veřejně v rámci celého týmu. Dotyčný by musel být skutečný rebel, aby to v něm nehrklo a nezačal více přemýšlet nad tím co a jak píše. Najednou bude jeho práce srovnávána s kolegy a nikdo nechce patřit mezi ty, kteří jsou hodnoceni špatně. Opět pozor na sociální stránku zavádění revizí. Ať se vám v týmu nezačne příliš jiskřit.

Jak vám to reviduje?

Moje osobní zkušenosti s revizemi kódu jsou téměř výhradně pozitivní. Postupně jsme si vyladili vývojový proces tak, že díky revizím kódu těžíme z výše popsaných přínosů. Hodně jsme se vzájemně naučili. Jakmile někdo objeví nový trik, hned ho umí i revidující. Dost nám pomáhají revize se softwarovými architekty z Německa, kteří mají dlouholeté zkušenosti s vývojem jejich systému, do kterého aktuálně píšeme moduly. Jejich rady nás mnohokrát nasměrovaly k lepšímu řešení.

Za mě tedy revizím kódu jednoznačné ANO.

A jaké máte zkušenosti s revidováním kódu vy? Dobré nebo špatné?

3 komentáře:

  1. Naše zkušenosti s CR jsou také pozitivní :-) Jen dvě poznámky/otázky:

    1) Trošku jsme bojovali s tím, kdo by měl dělat CR. Nejdříve jsme to měli nastavené tak, že autor kódu musel určit, kdo má dané CR udělat. Nakonec jsme to ale změnili tak, že autor kódu nemusí určovat reviewera (ale může). Takže nám to teď funguje tak, že když je člověk ve stavu, že by mohl udělat nějaká CR, tak se mrkne do Crucible a případně nějaká čekající CR udělá. Když o nějaké CR dlouho nikdo nejeví zájem, tak autor kódu určí, kdo má CR udělat.

    2) Jaké používáte na CR podpůrné nástroje? My máme Crucible od Attlasianu, což funguje moc pěkně. Ale víc by se mi líbilo něco přímo integrované do IDE (VS), protože když třeba někdo změní implementaci nějaké metody, tak bych se chtěl podívat, odkud všude se ta metoda volá - a to ve webovém Crucible jednoduše neudělám (a ve VS by to bylo easy). Takže to často u mě vypadá tak, že mám vedle sebe otevřené Crucible a VS.

    OdpovědětVymazat
    Odpovědi
    1. ad 1) U nás to zatím máme jednoduché - jsme v týmu pouze tři vývojáři. Dopředu neřešíme, kdo review udělá. Revidujeme všichni a kdo má zrovna čas, tak se k tasku přiřadí jako Reviewer a zreviduje ho. Jedeme takovým hybridním kanbanem, takže když se nahromadí příliš tasků v Ready for Review, tak polevujeme s kódováním a vrháme se na review.

      ad 2) Tasky evidujeme v YouTrack. Jako repository využíváme Mercurial. Do commit zpráv uvádíme odkaz na task z YT a strávený čas. Buildujeme přes TeamCity, kde máme speciální build, který odkazujeme z YT. S jeho využitím si YT namapuje změny v kódu (a strávený čas na úkolu) k příslušnému tasku. Review pak obvykle probíhá tak, že od tasku se doklikáme ke změnám mimo IDE. Ty umí TC docela hezky zobrazit, takže rozsah změny je pěkně dohledatelný. Když je potřeba jdu do VS a hrabu se přímo tam. Pokud se objeví drobnosti, pak je přímo revidující opraví a pošle task do Finished. Pokud jsou tam větší problémy, do kódu přidáme připomínky ve formě speciálního TODO komentáře, commitneme připomínky opět s odkazem na task. Task se vrátí do stavu Dev in Progress a autor se "nadšeně" vrhá na úpravy. :)

      Vymazat
    2. Tak to my děláme úplně stejně (oba body), jen místo YT máme JIRA, a hlavně máme navíc Crucible - ten nejen, že pěkně zobrazuje diffy, ale umožňuje jednotlivé commity sdružovat do jednoho CR, a umožňuje komentovat nejen na úrovni CR, ale i na úrovni souboru a dokonce jednotlivých řádků (využíváme nejčastěji). Takže člověk dokáže velmi bezbolestně poukázat třeba na nevhodné jméno lokální proměnné.
      Ptal jsem se hlavně proto, protože mě zajímalo, jestli používáte nějaký speciální nástroj na CR, a protože by mě zajímali reálné zkušenosti s něčím jiným, než je námi používaný Crucible.
      Třeba se chytne někdo jiný ;-)

      Vymazat