Přejít k navigační liště

Zdroják » Různé » Co nás naučilo Code Review

Co nás naučilo Code Review

Články Různé

Milion let nazpět opice slezly ze stromů, vyvinul se protistojný palec a nakonec se staly lidskými bytostmi. My se dívame na code review v podobném světle: něco, co odlišuje člověka od zvířete na loukách softwarové savany.

Nálepky:

V anglickém originále zveřejněno na blog.salsitasoft.com.

Nicméně, občas slyším takovéto komentáře od členů našeho týmu:

  • „Code review na tomto projektu jsou jen ztrátou času.“
  • „Nemám na code review čas.“
  • „Můj release je zpožděn, protože můj zrádný kolega mi ještě neudělal review.“
  • „Věřil bys tomu, že můj kolega chce, abych něco změnil ve svém kódu? Prosím, vysvětli mu, že křehká rovnováha vesmíru bude narušena, jestli se můj precizní, elegantní kód jakkoliv změní.“

Proč děláme revize kódu?

Připomeňme si, v první řadě, proč děláme code reviews. Jedním z nejdůležitějších cílů všech profesionálních vývojářů je neustále zlepšovat kvalitu svojí práce. I když je váš tým plný talentovaných programátorů, nemůžete se odlišit od schopného freelancera, pokud nebudete pracovat jako tým. Code review je jedním z nejdůležitějších způsobů, jak toho dosáhnout. Zejména:

  • druhý pár očí může najít závady a lepší způsoby, jak něco udělat
  • zajistí, že minimálně jedna další osoba je obeznámena s kódem
  • pomůže při vzdělávání nových zaměstnanců tím, že je vystavuje kódu zkušenějších vývojářů
  • podporuje sdílení znalostí dobrých postupů vzájemně mezi vývojáři
  • motivuje vývojáře, aby byli důkladnější ve své práci, protože vědí, že bude přezkoumána jedním z jejich kolegů

Dělat důkladné review

Nicméně, těchto cílů nemůže být dosaženo, pokud review není věnován odpovídající čas a péče. Jen proskrolovat patch a zkontrolovat formátování nepředstavuje důkladnou revizi kódu. Je možné chápat code review jako pair programming, což je docela populární praxe, ale přidává 100 % režii k času vývoje. Můžete strávit hodně času na review a stále spotřebovat mnohem méně celkového inženýrského času než při pair programmingu.

Mám pocit, že něco kolem 25 % původního vývojového času by mělo být vynaloženo na code review. Například pokud developer stráví dva dny implementací nějaké story, reviewer by na tom měl strávit zhruba čtyři hodiny.

Samozřejmě, že není tak důležité, kolik času s review strávíte, pokud ho děláte správně. Konkrétně, musíte pochopit kód, který přezkoumáváte. To neznamená jen, že znáte syntaxi jazyka, v kterém je kód napsán. To znamená, že chápete, jak kód zapadá do širšího kontextu aplikace, komponenty nebo knihovny, které je součástí. Pokud nechápete všechny důsledky každého řádku pak vaše review nebude velmi cenné. To je důvod, proč dobré review nemůže být provedeno rychle: Vyžaduje určitý čas prozkoumat všechny cesty, které vedou k volání nějaké funkce, zajistit, aby API třetích stran byly používány správně (včetně všech krajních případů) atd.

Kromě hledání defektů nebo jiných problémů v kódu bychom se měli ujistit, že:

  • Byly vytvořeny všechny potřebné testy.
  • Byla napsána příslušná design dokumentace.

Dokonce i vývojáři, kteří jsou dobří v psaní testů a dokumentací, si nemusí vždy vzpomenout je aktualizovat. Jemné šťouchnutí od reviewera ve správnou chvíli je někdy nezbytné, aby časem nezchátrali.

Předcházejte přetížení

Pokud váš tým dělá povinné revize kódu, je zde nebezpečí, že váš backlog nepřezkoumaných review přeroste až do bodu, kdy bude nezvladatelný. Pokud neuděláte žádné review dva týdny, můžete snadno strávit několik dní recenzováním. To znamená, že vaše vlastní produktivita bude mít velký a nečekaný výkyv, když se konečně rozhodnete se reviews vypořádat. To také velmi ztěžuje recenzování, jelikož správné revize kódu vyžadují intenzivní a trvalé duševní úsilí. Není lehké se takto soustředit celé dny.

Z tohoto důvodu by se vývojáři měli snažit vyprázdnit svůj review backlog každý den. Jeden přístup je řešit reviews jako první věc hned ráno. Děláním všech reviews ještě před vlastním vývojem zabráníte tomu, že se vám situace vymkne z rukou. Někteří preferují dělat reviews před nebo po obědě nebo na konci dne. Jakkoliv se rozhodnete, vždy je dobré uvažovat o code reviews jako součásti vašeho každodenního pracovního dne a ne jako o rozptýlení. Tímto zamezíte:

  • Nedostatku času na zvládnutí review backlogu.
  • Odložení releasu kvůli tomu, že reviews nejsou hotové.
  • Přezkoumávání kódu, který již není relevantní, protože se již dávno změnil.
  • Uspěchaným reviews, které recenzent dělal na poslední chvíli.

Psaní přezkoumatelného kódu

Recenzent není vždy ten zodpovědný za nekontrolovatelný backlog. Když můj kolega stráví týden přidáváním kódu chtě nechtě do všech částí nějakého velkého projektu, pak jeho patch bude opravdu těžké přezkoumat. Bude to příliš mnoho souvislostí k pochopení během jednoho sezení. Bude těžké pochopit smysl a příslušnou architekturu kódu.

To je jeden z mnoha důvodů, proč je důležité rozdělit svou práci do zvládnutelných jednotek. Používáme metodiku SCRUM, takže vhodná jednotka pro nás je story. Organizováním práce „by story“ a posíláním reviews vztahujících se pouze ke konkrétní story zajišťujeme, že kód je mnohem snazší přezkoumat. Váš tým může používat jinou metodiku, ale princip je stejný.

Existují i jiné předpoklady pro psaní přezkoumatelného kódu. Pokud nejsou všechna architektonická rozhodnutí zřejmá, pak dává smysl sejít se s recenzentem a předem je prodiskutovat. Díky tomu bude mnohem jednodušší pro recenzenta pochopit kód, protože bude vědět, čeho a jak se snažíte dosáhnout. To také pomáhá vyhnout se situaci, kdy budete muset přepsat velké kusy kódu poté, co recenzent doporučuje odlišný a lepší přístup.

Architektura projektu by měla být podrobně popsána. To je důležité tak jako tak, protože to umožňuje nového člena rychle zapracovat. Navíc to pomáhá recenzentovi. Unit testy jsou také užitečné pro ilustraci toho, jak by komponenty měly být použity.

Pokud vkládáte kód třetích stran, commitněte ho odděleně. Je mnohem těžší číst kód, když v půlce narazíte na 9000 řádků jQuery.

Jedním z nejdůležitějších prvků k vytvoření přezkoumatelného kódu jsou anotace. To znamená, že jako první projdete review sám a přidáte komentáře, kdykoliv máte pocit, že to pomůže recenzentovi pochopit, co se děje. Zjistil jsem, že anotace kódu zabere relativně málo času (často jen několik minut) a dělá obrovský rozdíl v tom, jak rychle a dobře může být kód přezkoumán. Samozřejmě, komentáře přímo v kódu mají mnoho stejných výhod, a měly by být používány tam, kde je to vhodné, ale často anotace v review dává větší smysl. Jako bonus, studie ukázaly, že vývojáři nachází mnoho defektů ve svém vlastním kódu během pročítání a anotování vlastního kódu.

Velké refaktoringy

Někdy je nutné refaktorovat kód způsobem, který ovlivňuje mnoho komponent. V případě velké aplikace to může trvat několik dní (nebo i více), výsledkem může být obrovský patch. V těchto případech může být standardní review nepraktické.

Nejlepším řešením je refaktorovat kód postupně. Udělat částečnou změnu přiměřeného rozsahu, který vede k fungujícímu kódu a posune vás směrem, kterým chcete jít. Poté, co tato změna byla dokončena a review vytvořeno, přistupte k další inkrementální změně a tak dále, až dokud není celý refaktoring dokončen. To nemusí být vždy možné, ale s trochou přemýšlení a plánování je obvykle realistické, aby se zabránilo masivním monolitickým patchům. Tento inkrementální postup může trvat déle, ale také vede k lepší kvalitě kódu, jakož i k jednoduššímu review.

Pokud opravdu není možné refaktorovat kód postupně (což asi říká něco o tom, jak dobrý a organizovaný byl původní kód), jedním z řešení je dělat pair programming namísto code review.

Řešení sporů

Váš tým se nepochybně skládá z inteligentních odborníků a téměř ve všech případech by mělo být možné dojít k dohodě, i když se názory na konkrétní otázku liší. Jako vývojář mějte otevřenou mysl a buďte připraven ke kompromisům, pokud váš recenzent preferuje jiný přístup. Nemějte proprietární postoj ke svému kódu a neberte si poznámky recenzenta osobně. Pokud má někdo pocit, že byste měli refaktorovat nějaký duplicitní kód do opakovaně použitelných funkcí, neznamená to, že jste méně atraktivní, brilantní a okouzlující osoba.

Jako recenzent, buďte taktní. Před navrhování změn zvažte, zda je váš návrh opravdu lepší, nebo je to jen věc vkusu. Budete mít větší úspěch, pokud si chytře vyberete své bitvy a budete se soustředit na oblasti, které jednoznačně vyžadují zlepšení. Říkejte věty jako „mohlo by být vhodné zvážit…“ nebo „někteří lidé doporučují…“ místo „i můj křeček to dokáže napsat efektivněji.“.

Pokud opravdu nemůžete najít střední cestu, požádejte třetího vývojáře, kterého oba respektujete, aby se podíval a vyjádřil svůj názor.

Salsita Software

Salsita Software je softwarová společnost, která se specializuje na vývoj komplexních moderních webových a mobilních aplikací. Sponzorujeme JavaScripting.com, komunitní portál, který pomáhá vývojářům hledat knihovny a frameworky pro JavaScript.

Přeložil: Luboš Turek

Komentáře

Subscribe
Upozornit na
guest
12 Komentářů
Nejstarší
Nejnovější Most Voted
Inline Feedbacks
View all comments
omelkes

… práci do zvládnutelných jednotka. -> do zvládnutelných jednotek.

Jinak díky za zajímavý článek.

EdaCZ

Ahoj.
našel jsem pár chybek.

Pokud nechápete všechny důsledky každého řádku pak vaše review nebude
velmi cenné.

Chybí čárka před slovem pak.

API třetích stran byli používány správně

Chyba v i.

Jemné šťouchnutí od reviewera ve správnou chvíli je někdy nezbytné,
aby časem nezchátrali.

Nezchátral*y*, pokud jsou tedy myšleny ty dokumentace.

…když se konečně rozhodnete se reviews vypořádat.

S reviews, nikoliv se.

…že se vám situace vymykne z rukou.

Vymkne.

…vždy je dobré uvažovat o code reviews jako součásti vašeho
každodenního pracovního dne a ne jako o rozptýlení.

Před „a ne“ bych dal čárku. To „a“ lze případně úplně vypustit.

…přistupte k další inkrementální změně a tak dále až dokud není celý
refaktoring dokončen.

Čárka před „až“.

Snad jsem už nic gramatického nepřehlédl. Dále by se tam našlo dost míst, která by šla přeformulovat stylisticky lépe, ale to už jsou detaily.

Bohužel už patří ke koloritu Zdrojáku, že články, ač informačně a věcně velmi přínosné, nejsou jazykově zpracované příliš pečlivě. Mám pocit, že podobný jazykový příspěvek píšu ke každému druhému článku.

Pokud byste měli zájem, můžu na nové články kouknout z pohledu korektora ještě předtím, než je uveřejníte, ať se vyhneme takovému množství chyb, které bylo například v tomto článku.

PS: Nic ve zlém. Jsme lidi… :-)

PS2: Editor příspěvků neinterpretuje dobře odřádkování. Za slovem „Ahoj“ mám odřádkování, ve výsledném příspěvku tomu tak ale není.

Eda

EdaCZ

Tak ta nepřesná interpretace odřádkování je jen v náhledu příspěvku, po odeslání už je to ok.

Opi

Tady je krasne videt, jak je dulezity druhy par oci :)

EdaCZ

Ano :-)

EdaCZ
uetoyo

@EdaCZ
„API třetích stran byli používány správně“
Proč je tam špatně i? To API nemusím skloňovat… pokud si chci nějak pomoci tak to aplikační rozhraní.

makovec

„Ta rozhraní“, takže nejlépe asi „API třetích stran byla používána“?

Tomáš Tintěra

Zajímala by mne konkrétní data, čeho je běžné nebo čeho lze pomocí code review dosáhnout. Snížení chybovosti, zlepšení zastupitelnosti mezi členy týmu, zlepšení čitelnosti kódu a vnitřní komunikace.

V článku jsou chyby, viditelné na první přečtení. Je vidět, že by mu pomohlo review. Alespoň standardní jazyková korektura. Každopádně: díky za článek.

Podle mé zkušenosti na review často stačí i méně času. (Měli jsme jednoduchou aplikaci. DB a web, pět vývojářů, projekt běžel jednotky měsíců).

Enum a statická analýza kódu

Mám jednu univerzální radu pro začínající programátorty. V učení sice neexistují rychlé zkratky, ovšem tuhle radu můžete snadno začít používat a zrychlit tak tempo učení. Tou tajemnou ingrediencí je statická analýza kódu. Ukážeme si to na příkladu enum.