Smysl revizí kódu

Dnešní příspěvek je – očividně – o revizích kódu. Nevím, jak moc toto téma znáte – možná žádné revize neděláte, možná je používáte mnohem lépe než já, takže nejdřív popíšu své zkušenosti.

Co jsem tak zatím viděl v implementacích Dynamics AX, obvyklý proces je naprogramovat kód, nechat ho otestovat konzultantem a poslat zákazníkům (to platí i pro ISV). Nikdo se nikdy nepodívá do kódu, aby viděl, zda je naprogramován správně, takže není žádným překvapením, že většina implementací má problémy s vnitřní kvalitou, například s udržovatelností. Tyto věci se zkrátka vůbec netestují.

Zavedení revizí kódu může znamenat veliký rozdíl v celkové kvalitě, takže já je silně propaguji a chci věřit, že všechny minulé týmy ocenily jejich výsledky.

Nicméně se ukazuje, že není vždy všem jasné, co to revize kódu jsou a proč jsou důležité.

Smysl revizí kódu

Mohli byste očekávat, že smysl revizí je zřejmý, ale realita ukazuje něco jiného.

Já vidím několik důležitých důvodů pro revize:

  1. Testování vnitřní kvality – udržovatelnost, testovatelnost atd.
  2. Funkční testování
  3. Sdílení znalostí

Myslím, že všechny tyto věci jsou skutečně důležité. Ale některé týmy degradují revize kódu na pouhé kontrolování jmenných konvencí a odsazení kódu; ani jejich způsob provádění revizí nedovoluje nic moc lepšího (například: pokud se podíváte na pár řádek kódu promítnutých na plátno, jak můžete říct, zda ten kód měl být vůbec napsán?). Dělají “revize kódu”, ale nezískají z nich téměř nic. Docela škoda.

Pojďme se podívat na účel revizí podrobněji.

Vnitřní kvalita

Testování interní kvality (“kvality kódu”) je typicky hlavní důvod, proč vývojové týmy začínají s revizemi kódu. A to je v pořádku – protože tento typ testování často úplně chybí, je zaplnění této mezery klíčové.

Bez snahy o úplnost mohu jmenovat pár důležitých věcí k otestování:

  • Celkový přístup – dělá kód správnou věc a používá správné prostředky? (Pokud ne, můžeme skončit s krásnou implementací velmi špatného nápadu.)
  • Udržovatelnost/rozšiřitelnost – jsme schopni snadno rozšiřovat dané řešení, opravovat chyby atd.? (Toto zahrnuje mnoho základních programátorských pravidel – např. DRY or SRP, OO design, design  DB apod.)
  • Testovatelnost – jsme schopni kód otestovat?
  • Dokumentace – jsme schopni porozumět proč byl kód vytvořen a co dělá?
  • Výkonnost – je návrh v pořádku z pohledu výkonu?  (V AX to zejména znamená optimalizaci klient/server volání. Nejde zde ale o testování výkonu.)

Většinu z těchto věcí nelze testovat obvyklým testováním černé skříňky, nebo pouze když způsobí opravdu velký problém (např. výkon) a to je obvykle dost pozdě. Pokud tento typ testování chybí, lze se snadno dostat do problémů s udržovatelností, testovatelností, výkonem… (Jsem si jistý, že tohle je řadě AX týmů povědomé.)

Funkční testování

Není neobvyklé slyšet, že revize kódu nemají testovat, zda kód dělá to, co by měl. Toto zvláštní tvrzení má (opět) kořeny v představě, že revize by měly kontrolovat pouze věci jako odsazení.

Je důležité si uvědomit, že revize kódu je typ testování. Jde o testování bílé skříňky, tudíž vidíme o něco víc než testeři černé skříňky a tuto schopnost musíme využít k nalezení problémů v produktu.

Některé chyby jsou daleko lépe viditelné v kódu – například můžete okamžitě vidět, že nějaká potenciální chyba není ošetřena. Funkční testeři nemusí o takové možnosti vůbec vědět (např. neví, že se nějaké hodnoty čtou z potenciálně chybějícího souboru).

Teorie testování často mluví o testování mezních hodnot, analýze cest v kódu a takových věcech. Toto zjevně nelze dělat skrz testování černé skříňky. Když začnete uvažovat všechny kombinace průchodu skrz několik metod, rychle také zjistíte, že je téměř nemožné otestovat skutečně všechny možnosti z UI. (V této situaci se ukáže nenahraditelnost jednotkového testování.)

Navíc některé podmínky (např. race conditions nebo stav externích systémů) je pro funkční testery téměř nemožné nasimulovat, ale mohou být snadno připraveny kouskem kódu.

Je vidět, že podstatná část testování musí být provedena pomocí bílé (nebo šedé) skříňky a postup obvykle používaný ve světě Dynamics AX není vyhovující. Testování je většinou prováděno byznys konzultanty kteří často vědí velmi málo o testování softwaru. Ale aspekty popsané výše stejně musí být otestovány jiným typem testera – někým kdo dokáže vidět do kódu, identifikovat průchody kódem, simulovat požadované podmínky atd. Microsoft má pro tento účel SDETy (Software Development Engineer in Test), my to obvykle musíme nechat na zkušených vývojářích revidujících kód.

Sdílení znalostí

Sdílení znalostí není obvykle primární důvod pro revize kódu, ale to neznamená, že není důležité. Má různou formu:

  • Revidující vývojáři se seznámí s novým kódem, takže jeho znalost není omezena jen na autora.
  • Diskuze mezi vývojářem a inspektorem umožňují sdílení znalostí o problému a možných řešeních.
  • Opakující se chyby ukazují, kde je potřeba dodatečný trénink.
  • Vývojáři začínají chápat, jak důležité je psát čitelný kód.

Zde bych chtěl zdůraznit, jak užitečné je revidovat kód psaný nezkušenými vývojáři a věnovat čas na vysvětlení proč a jak řešit některé problémy. Tyto diskuze nad reálným kódem a porovnávání jejich řešení s lepším přístupem se ukázaly jako velmi prospěšné.

Měl by být revidován všechen kód?

Tato otázka se objevuje dost často, ale já si nemyslím, že je složitá. Samozřejmě nemusíte testovat kód doručovaný zákazníkům, ale nebuďte překvapeni následky.

Je velice riskantní dodávat neotestovaný nebo špatně otestovaný kód a čekat než dojde k poškození dat v systému zákazníka nebo jiném ošklivému incidentu. Tudíž veškerý kód by měl projít revizí (= statickým testováním bílé skříňky). To zahrnuje i několikařádkové opravy (které jsou ve skutečnosti velmi náchylné k chybám) a kód opravující problémy nalezené předchozími revizemi.

Nástroje

Vždy je dobré zautomatizovat činnosti, které nemusí být prováděny manuálně. Nástroje pro statickou analýzu jsou schopné najít mnoho potenciálních problému automatizovaně, čímž šetří čas a zajišťují, že nic není jednoduše přehlédnuto. V Dynamics AX dělá takovou analýzu Best Practice Check, do kterého můžete snadno přidat nové kontroly pokrývající vaše specifické potřeby.

To je má obvyklá odpověď těm, kteří věří, že primární smysl  revizí kódu je zkontrolovat, že všechny objekty používají firemní prefix – to celé lze snadno implementovat jako BP check, takže není důvod s tím trávit více času než vyžaduje spuštění kontroly. Vývojáři revidující kód by se měli soustředit na ty věci, které nelze zautomatizovat.

Načasování

Ačkoli jsem zmínil revizi celkového přístupu, určitě se chcete vyhnout problémům v takových věcech, když už je veškerý kód napsaný, protože to může znamenat, že bude muset být celý vyhozen. Je tudíž mnohem efektivnější revidovat zásadní věci dříve, například v samostatné fázi technického designu, zběžných revizích nedokončeného kódu a tak dále. To je zvláště důležité, pracujete-li s nezkušenými vývojáři.

Překvapivě často lidé zapomínají vyhradit čas na opravy a tak nejsou nalezené problémy vůbec řešeny (nebo alespoň odloženy do další verze). To drasticky snižuje hodnotu revizí kódu. Děje se to, protože revize jsou považované za něco volitelného, něco bez velkého dopadu na finální produkt. To může být pravda pokud se kontrolují jen pojmenování apod., ale náležité revize zvládnou mnohem více. Moje doporučení je považovat revize kódu za povinnou část fáze vývoje a neposílat žádný kód do regulérního funkčního testování dokud není zkontrolován a opraven. Je to také proto, že změny v kódu kvůli revizím jsou dost časté a bylo by plýtvání časem několikrát opakovat funkční testování (což často znamená i nové nasazení na testovací prostředí).

Vlastní pravidla Best Practice

Pokud by byl zájem, můžu věnovat nějaký z dalších příspěvků tomu, jak naprogramovat vlastní pravidlo do Best Practice Checku.