Není revize kódu jako revize kódu

Dagi se nám nějak rozepsal. Takže abych s ním udržel krok, napíšu velmi opožděnou reakci na to co napsal skoro před rokem a dneska mi připomněl svým nejnovějším zápiskem.

Revizi kódu můžeme pojmout několika způsoby. Zažil jsem jednu čistě formální revizi, která se dělala jen proto, aby se odškrtl chlívek v nějakém předávacím protokolu. Vypadá to tak, že vezmete svůj kód, předáte ho někomu koho jste nikdy neviděli a on vám pošle protokol s výsledky. Protože obvykle recenzent nemá moc času, tak to jen tak prolétne, napíše vám že tady měly být tři mezery místo čtyř a tato proměnná měla mít tři slabiky. Obvykle není přínos moc velký. Tedy kromě toho, že se splní smlouva, což je samozřejmě důležité. Někdy můžou být výsledky i docela zajímavé. Pamatuji se, že mi při jednom code review vytkli, že jsem použil návrhový vzor singleton. No nakonec jsem si ho obhájil. Toho, že jsem o kus dál dělal docela divočárny s Java reflection si nikdo kupodivu nevšiml.

Takováto revize kódu je z pohledu programátora k ničemu. Nicméně to není jediná možnost jak je dělat. Můžeme dělat i neformální revize kódu. Mám s nimi docela dobrou zkušenost. A nejsem sám. Jak taková revize vypadá?

Je to jednoduché. Vybere se kus kódu, který je nějak rozumně ohraničený a přiměřeně velký (všimněte si mého exaktního jazyka). Jeho autor ho připraví na revizi, může ho prohnat automatickou kontrolou jako třeba FindBugs a podobně. Pak ho dostanou do ruky recenzenti (obvykle dva), kteří mají za úkol udělat samotnou revizi. Ale pozor, recenzenti musí být s autorem kódu členy stejného týmu. To je nesmírně důležité. Tím že dělají revizi kódu se ho mimochodem i naučí. Takže místo jednoho člověka, který zná danou část aplikace už máme tři lidi, kteří se s ní kamarádí.

A to není všechno. Celá revize by měla být završena sezením. To se sejde celý tým v zasedačce, pouští si kód na projektoru a povídá si o něm. Recenzenti ukazují co se jim líbilo, co by vylepšili a všichni dohromady pojídají koláč. Je nesmírně důležité udržet neformální a přátelskou atmosféru. Pokud máte manažera, který nechápe, že se při programování dělají chyby, na sezení ho nepouštějte. Je také dobré mít moderátora a zapisovatele. Moderátor řídí diskuzi, dohlíží na to aby se sezení nezvrhlo v hádku o tom, jestli je lepší HashMap nebo TreeMap. Zapisovatel zapisuje rozhodnutí o tom co je potřeba na kódu ještě vylepšit. Podobná revize kódu vám přinese několik výhod.

  1. Autor kódu se víc snaží. Znám to ze své zkušenosti. Když vím, že po mě někdo bude kód číst, tak se prostě víc snažím. Odpustím si některé prasárny, víc se zamýšlím nad tím co dělám. Je to hloupé, ale je to tak. (Alespoň se nestydim to přiznat)
  2. Šíří se znalosti. To je podle mě největší přínos revizí. Jak recenzenti, tak autor kódu, ale i ostatní členové týmu se od sebe navzájem hrozně moc naučí. Při sezení si povídají nad konkrétním kódem, můžou si ukázat jak autor super použil návrhový vzor, jak by mohl lépe použít knihovnu XYZ apod.
  3. Občas se při revizi najdou i nějaké chyby.

Revize kódu mají i své nevýhody. Pokud je v týmu nezdravá atmosféra, můžou se zvrhnout v ošklivou hádku. Od toho máme moderátora aby tomu zabránil. A koláč, ten dokáže atmosféru docela uvolnit. Samozřejmě největší nevýhodou revizí je to, že žerou čas. Stejně jako dobrý návrh a unit testy, tak i revize kódu zabírají čas. A toho není nikdy nazbyt. Tady mi nezbývá než použít svůj oblíbený citát.

Nikdy není dost času udělat to pořádně, vždycky je dost času udělat to znovu.

7 Responses to “Není revize kódu jako revize kódu”

  1. Honza Novotný Says:

    Moc hezky shrnuto.

    Bohužel v současné době např. u nás děláme review pouze nad veřejným API a hrubou architekturou knihoven. Na revize vnitřního kódu prostě zatím není ve většině případů čas. Obvykle se spoléháme na to, že ze zkušenosti víme, že kolegové prostě píší dobrý kód.

    Jiná situace je s nováčky v týmu. Tam jsou review v podstatě nevyhnutelné a potvrzuji, že je to neuvěřitelný žrout času.

  2. Dagi Says:

    Lukasi diky za tip, ten kolac zkusim nejak zaridit ;-).

  3. Lukas Says:

    Code Review zrout casu nejsou, naopak je to velmi efektivni zpusob jak najit defekty v kodu.

  4. tommy Says:

    Dle mych zkusenosti code review zabere cca 50% casu (a vice) co trvalo kod napsat (v pripade ze kod napsal jeden, review provadi 3). Je to vec prospesna a uzitecna, ale casove narocna a unavujici. Jen tezko udrzite po celou dobu (napr. 4 hodiny) plnou pozornost vsech zucastnenych. Caste prestavky take nejsou prilis efektivni. Kolac je prima, ale dovedu si predtavit jak po 2 hodinach zucasteni zacnou premyslet zda se soustredit na kolac ci na reviewovany kod 🙂

  5. Lukáš Křečan Says:

    Proto jsem psal, že by ten kód měl být rozumně velký. Doba sezení by se měla pohybovat mezi jednou až dvěma hodinami.
    A znova opakuji, podle mého názoru je hledání defektů jenom taková záminka jak si popovídat o kódu a něco se naučit.

  6. Luboš Račanský Says:

    Stejně jako platí rčení: "Nejsme tak bohatí, abychom si mohli kupovat levné věci." by se říct: "Nemáme tolik času, abychom mohli ignorovat unit testy a code review." Protože ono se nám to vymstí a onen zdánlivě ušetřený čas se nakonec několikanásobně prodraží.

  7. Jan Kotek Says:

    Neni to hrozne formalni? Podle me staci projit nove commity a pripadne soukromne mail s cislou radku. Funguje to skvele 🙂