Egy code review lista, hogy a fontos részekre tudj koncentrálni

Egy code review lista, emellett a code review köré húzott szabályok és irányelvek sokkal előnyösebbé tehetik a code review-t a csapatod számára, és jelentősen fel is gyorsíthatják azokat.

Tanulmányokban kimutatták, hogy azok, akik használnak egy ellenőrzőlistát code review során sokkal jobban teljesítenek, mint azok, akik nem. Szóval nem árt elgondolkodnod egy ilyen lista összeírásán, akár új vagy a szoftverfejlesztésben, akár már jóval tapasztaltabb.


Legyél a saját bírád


A code review ellenőrzőlista nem csak a code review kollégáknak hasznos. A kódváltozás szerzőjeként kövesd a code review best practice-t, és legyél előbb a saját bírád!

Szóval mielőtt elküldöd a kódod elemzésre, menj biztosra, hogy:

•A kód lefordul és warning nélkül átmegy a statikus analízisen.

•Átmegy minden teszten (unit, integrációs és rendszertesztek).

•Kétszer is átnézted elgépelések után kutatva, és feltakarítottál magad után (kommentek, todok, stb.)

•Körülírtad, hogy miről szól a változtatás, beleértve az okát, és hogy pontosan mi változott.

Ezek mellett a kód szerzőjeként végig kellene menned ugyanazon a listán, amit review során is követnek.



Ellenőrzőlista a code reviewereknek


Code reviewerként a feladatod először a legfontosabb hibák után kutatni. Nyilván könnyebb elveszni a szőrszálhasogatásban, de azt semmiképp nem szabad hagyni. Az egyik nagyobb kutatásban, amit a Microsoftnál folytattunk, azt nyomoztuk, hogy hogyan néz ki egy igazán jó code review. Egyértelműen látszott, hogy a nagyobb strukturális vagy logikai problémákat felfedő kommentek sokkal hasznosabbak, mint az apró problémákra fókuszálók.

És itt jön a képbe az ellenőrző lista. Egy jó lista a legfontosabb problémákra irányítja a figyelmed. Lentebb találsz egy listát, amit én is használok a code review workshopok során. Tíz különböző részre van felosztva, minden rész számos kérdést tesz fel. Kezdjük is:



Implementáció


•Ez a változás tényleg azt csinálja, amit kell neki?

•Le lehetne egyszerűsíteni ezt a megoldást?

•Hoz a változtatás felesleges fordítási vagy futási idejű függőségeket?

•Használtak olyan keretrendszert, API-t, könyvtárat vagy szolgáltatást, amit nem kellett volna?

•Lehetne használni valamilyen keretrendszert, API-t, könyvtárat vagy szolgáltatást, ami javíthatná a megoldást?

•Megfelelő absztrakciós szinten van a kód?

•Eléggé moduláris a kód?

•Meg tudnád oldani a problémát másképp, ami lényegesen jobb lenne a kód fenntarthatóságának, olvashatóságának, teljesítményének és biztonságának szempontjából?

•Létezik már hasonló funkcionalitás a kódbázisban? Ha igen, akkor miért nem az van újrahasználva?

•Van bármilyen best practice, tervezési minta vagy nyelvspecifikus minta ami lényegesen javíthatná a kódot?

•Követi a kód az olyan objektumorientált tervezési elveket, mint az egyszeres felelősség elve, nyitva zárt alapelv, Liskov-féle helyettesíthetőségi alapelv, interfészek szétválasztásának elve és a függőséginverzió?


Logikai hibák és bugok


•Eszedbe jut olyan use case, ahol a kód nem az elvártnak megfelelően viselkedik?

•Tudnál olyan inputot vagy külső eseményt mondani, ami eltörné a kódot?


Hibakezelés és logolás


•Megfelelően van megcsinálva a hibakezelés?

•Kellene bármilyen logging vagy debugging információt hozzáadni vagy elvenni?

•Felhasználóbarátok a hibaüzenetek?

•Van elég logolt esemény és könnyű debugolást lehetővé tevő módon vannak megírva?


Használhatóság és hozzáférhetőség


•Használhatósági szempontból jól van megtervezve a megoldás?

•Jól dokumentált az API?

•Elérhető a javasolt megoldás (UI)?

•Intuitív az API/UI használata?


Tesztelés és tesztelhetőség


•Tesztelhető a kód?

•Van elég automatizált tesztje (unit/integrációs/rendszerteszt)?

•A létező tesztek megfelelően lefedik a változást?

•Van még olyan teszt, input vagy edge case amit ezen felül le kéne tesztelni?



Függőségek


•Megkövetel a változás kódon kívüli változtatásokat, mint például a dokumentáció, konfiguráció vagy readme fájlok frissítése? Ha igen, ezek meg lettek csinálva?

•Van bármilyen következménye a változásnak a rendszer más részeire és a fordított kompatibilitásra nézve?


Biztonság és adatvédelem


•Hoz a kód biztonsági sebezhetőségeket a szoftverbe?

•Megfelelően van kezelve az autorizáció és az autentikáció?

•Biztonságosan kezeli és tárolja az érzékeny adatokat, mint például a felhasználói adatok és a bankkártyainformációk?

•Megfelelő titkosítást használ?

•Felfed a változás titkos információkat (kulcsok, felhasználónevek, stb.)?

•Ha a kód kezel felhasználói inputot, kezeli az olyan biztonsági kockázatokat, mint a cross-site scripting, SQL injection, megfelelően kezeli és validálja a bemenetet?

•Ellenőrzi a külső API-tól vagy könyvtárakból érkező adatokat megfelelően?


Teljesítmény


•Úgy gondolod, hogy a változás negatívan fogja befolyásolni a rendszer teljesítményét?

•Látsz bármilyen lehetőséget, amivel javítani lehetne a kód telejsítményén?


Olvashatóság


•Könnyű volt megérteni a kódot?

•Melyik részek voltak zavarosak, és miért?

•Kisebb módosításokkal javítható az olvashatóság?

•Más függvény, illetve változónevekkel javítható lenne az olvashatóság?

•A megfelelő fájlban/mappában/package-ben található a kód?

•Szerinted vannak olyan függvények, melyeket újrastrukturálva intuitívebb lenne a vezérlési folyam?

•Érthető az adatfolyam?

•Vannak felesleges kommentek?

•Van olyan komment, ami jobban is közvetíthetné az információt?

•Több komment használatával érthetőbb lenne a kód?

•El lehetne távolítani néhány kommentet a kód érthetőbbé tételével?

•Van bárhol kikommentelt kód?


Szakértői vélemény


•Szerinted van olyan konkrét szakember, mint például egy biztonsági vagy használhatósági szakértő, akinek át kellene néznie a kódot mielőtt azt commitolni lehetne?

•Fog a változás más csapatokat is befolyásolni? Kellene, hogy megkérdezzük őket róla?


Ennyi lenne. Átnézted és átgondoltad a legnyomósabb problémákat. Gratulálok!

Most pedig elvégzünk egy gyakorlatot, amit a code review workshopjaim során szoktam csináltatni a résztvevőkkel. Válaszold meg az alábbi három kérdést:

1.A lista melyik részére fektetted a legnagyobb hangsúlyt?

2.Melyik részét hanyagolod el a leginkább?

3.Szerinted vannak olyan részek, amik fontosabbak, mint mások? Miért?



De mi lesz a kódolási stílussal és konvenciókkal?


Talán a feladat során rájöttél, hogy nem ellenőrzöm, hogy a kód követi-e a megfelelő kódolási stílust. Szóval az nem lenne fontos?

Röviden, természetesen az is fontos. A kristálytiszta kódolási irányelvek alkalmazása az egyetlen módja, hogy konzisztenciát kényszerítsünk ki egy kódbázisban. A konzisztencia pedig meggyorsítja a code review-t, lehetővé teszi a projektek könnyű módosítását és elősegíti az olvashatóságot és a karbantarthatóságot.

A Google egy jó példa arra, hogy hogyan kell ezt jól csinálni. Ez pedig lehetővé teszi számukra, hogy nekik legyen a legrövidebb átfutási idejű code review rendszerük.

Kiindulási pontnak tudom javasolni a Google által sok nyelvhez meghatározott kódolási stílusokat.

Fontos, hogy lefektessük az alapszabályokat, de ezt egyszer s mindenkorra tegyük meg, ne vitatkozzunk róla folyamatosan.


A kristálytiszta kódolási stílus felgyorsíthatja a code review-t. De csak akkor, ha automatikusan kikényszeríted őket különböző eszközök segítségével.


Automatizálj mindent, amit csak lehet


Amint eldöntötted, hogy kellene kinéznie a kódbázisnak, szánj időt rá, hogy telepíts és konfigurálj eszközöket, melyek egy gombnyomással megfelelően formázzák a kódot. Emellett annyi mindent tehetsz még. Használj statikus analízist végrehajtó eszközöket, hogy ennek a terhét levedd a code reviewereid válláról. Megéri az erőfeszítést.


A reviewerek ne kutassanak olyan hibák után, amit eszközök sokkal megbízhatóbban és költséghatékonyabban megtalálnak.



Legyél tiszteletteljes, alázatos és kedves


Végezetül, a code review visszajelzése nem csak azon múlik, hogy MIT mondasz, hanem hogy azt HOGYAN teszed. A legjobb code review sem ér semmit, ha nincs gondosan, alázatosan és kedvesen megfogalmazva. Kezdetnek, fogalmazd meg a visszajelzésed javaslatként követelés helyett. Például ahelyett, hogy „A változót removeObejct-nek kellene hívni.” mondd azt, hogy „Mi lenne, ha a változót removeObject-nek hívnánk?” Ha erre kíváncsi vagy részletesebben is, akkor olvasd el a cikkem arról, hogy hogyan fogalmazzuk meg a code review visszajelzést tiszteletteljesen.


(Forrás)


***


Ha Te is kreatív, kihívásokkal teli mérnök állást keresel minőségi munkáltatónál, jó helyen jársz, mert a Schönherz Bázis épp azért jött létre, hogy Neked segítsen.
Gyere, nézz szét aktuális állásaink között!


2020.05.18.