TSM - Code review
Mădălin Ilie
- Cluj Java Discipline Lead
Code Review-ul este o examinare sistematică a codului sursă. Scopul acestui proces este identificarea și corectarea problemelor trecute cu vederea în faza inițială de scriere a codului, îmbunătățind în același timp calitatea codului cât și abilitățile dezvoltatorilor.
De ce este Code Review-ul important
Steve McConell prezintă în cartea sa Code Complete (http://www.amazon.com/Code-Complete-Practical-Handbook-Construction/dp/0735619670) câteva argumente foarte bune despre eficiența procesului de Code Review:
Testarea softului în sine are eficacitate limitată - rata medie de detectare a defectelor este de 25% pentru unit testing, 35% pentru testarea funcțională și 45% pentru integration testing. În opoziție, eficacitatea Code Review-ului si Design Review-ului este între 55%-60%. Studiile de caz pentru revizuirea rezultatelor aplicării Code Review-ului au fost impresionante:
- Dintr-o serie de 11 proiecte software dezvoltate de aceeași echipă, primele 5 au fost dezvoltate fără a avea procesul de Code Review, iar următoarele 6 au fost dezvoltate folosind și procesul de Code Review-ul. După ce toate cele 11 proiecte au ajuns în producție, primele 5 au avut o rată de 4.5 erori pe 100 de linii de cod. Următoarele 6 au avut doar 0.82 de erori pe 100 de linii de cod. Review-urile au îmbunătățit rata de erori cu 80%.
- Aetna Insurance Company a găsit 82% din erori cu ajutorul Code Review-ului ceea ce le-a permis să scadă și numărul de resurse alocate dezvoltării cu 20%.
- Proiectul Orbit de la IBM utiliza 11 nivele de review. A fost livrat la timp și a avut doar 1% din numărul de defecte care în mod normal ar fi apărut.
- Un studiu realizat la compania AT&T a arătat că după introducerea review-urilor numărul de defecte a scăzut cu 90%, iar productivitatea a crescut cu 14%.
- Jet Propulsions Laboratories estimează că economisesc 25.000$ per review.
Motivele principale pentru care se face Code Review-ul sunt:
- Găsirea și fixarea defectelor devreme în procesul de dezvoltare;
- O mai bună înțelegere comună a codului sursă;
- Ajută la menținerea unui nivel de consistență în design și implementare;
- Ajută la identificarea defectelor comune în echipă si la reducerea timpului de fixare;
- Ajută la creșterea încrederii stakeholder-ilor asupra calității procesului de devoltare;
- O înțelegere comună și uniformă a codului ajută și la reducerea impactului în proiect atunci când anumiți membrii ai echipei devin indisponibili;
- O perspectivă nouă. O altă "pereche de ochi" crește gradul de obiectivitate. La fel ca și în cazul separării între dezvoltare și testare, Code Review-ul introduce distanța necesară pentru a recunoaște mai ușor problemele;
- Mândrie/recompensă. Recunoașterea abilităților este o recompensă semnificativă pentru mulți programatori;
- Coeziune mai bună a echipei.
Pentru a înțelege cât de important este să fixăm cat mai multe defecte încă din faza de dezvoltare, aveți mai sus un grafic despre costurile unui defect în diferite faze ale produsului:
Ariile principale pe care se concentrează Code Review-ul:
- Unit testing,
- Documentarea codului și convenții de codare,
- Tratarea erorilor,
- Resource leaks,
- Thread Safety,
- Structuri de cotrol,
- Performanță,
- Funcționalitate,
- Securitate,
- Consistență cu designul și arhitectura existentă
- Utilizarea corectă a librăriilor externe
Roluri și responsabilități
Dezvoltatorul: este persoana care a scris codul ce urmează a fi revizuit și este cel care inițiază cererea pentru Code Review.
Reviewer/s: sunt persoanele care vor revizui codul și vor raporta problemele găsite devoltatorilor.
La fel ca orice altă abilitate, reviewer-ii devin mai buni făcând foarte multă practică. Iată câteva ponturi care vă vor ajuta să porniți pe drumul cel bun.
Sfaturi pentru Dezvoltarori
- Primul reviewer este chiar autorul codului i.e. TU
- Creează un checklist cu lucrurile pe care se focusează cel mai mult procesul de Code Review. O astfel de listă ar trebui să fie destul de simplu de făcut. De obicei, se urmăresc ideile cele mai importante din standardele de codare. Pentru că este o listă personală, te poți concentra pe ariile pe care știi că ai cele mai mari probleme și poți omite ariile despre care știi că te descurci foarte bine. Nu doar că vei reduce numărul de probleme găsite, dar vei face procesul de review unul foarte scurt, ceea ce va face pe toată lumea fericită.
- Tu și codul sunteți 2 entități diferite. Ține minte că scopul principal al review-ulu este găsirea de probleme și în 99% din cazuri vor fi găsite probleme. Nu lua lucrurile personal. Unii dezvoltatori au un simț al proprietății și un orgoliu foarte mare, ajungând chiar să se identifice cu codul scris.
- Întelege și acceptă faptul că vei face greșeli. Scopul este să le găsești cât mai devreme și să nu ajungă în producție. Ține minte că doar facând greșeli poți să evoluezi.
- Indiferent de cât de multe "karate" știi, există mereu cineva care sție mai multe. O astfel de persoană te poate învăța câteva "mișcări" interesante. Caută și acceptă ideile celorlalți, mai ales când crezi că nu ai nevoie de ele.
- Nu rescrie cod fără a te consulta și cu echipa. Este o diferență foarte mică între a fixa codul și a rescrie codul. Învață diferența și încearcă să faci aceste schimbări în cadrul procesului de Code Review. Nu începe să refactorizezi de unul singur crezând că ai dreptul să faci asta și că ești cea mai bună persoană din echipă, iar restul sunt niște ignoranți.
- Singura constanță este schimbarea. Fii foarte deschis și acceptă lucrul acesta. Cu atât accepți mai repede, cu atât ești mai fericit. Privește fiecare schimbare a cerințelor, platformei, sau tool-urilor ca o nouă provocare.
- Luptă pentru ceea ce crezi, dar acceptă cu grație înfrângerea. Acceptă faptul ca uneori ideile tale vor fi respinse. Chiar de se dovedește că ai dreptate evită să spui "Ți-am spus eu".
- Nu fi "the guy in the room". Nu fi persoana care stă în colțul lui și singura dată când se ridică de la birou este atunci când merge să iși ia o cafea sau o cola. O astfel de persoană nu se va integra niciodata într-un mediu colaborativ și îi va foarte greu să accepte remarcile rezultate în procesului de Code Review.
- Întâlnirile de Code Review nu sunt întâlniri pentru rezolvare problemelor. Scopul lor nu e să discutăm toate probleme ce există pe proiect, ci doar punctual review-ul aflat în desfășurare.
- Ajută la menținerea standardelor de codare. Oferă-ți ajutorul pentru a adăuga în standardele de codare lucruri care apar la Code Review dar nu sunt încă trecute și în standardele de codare.
Sfaturi pentru Revieweri
- Critică codul, nu persoanele - fii blând cu dezoltatorii, nu cu codul. Pe cât posibil comentariile tale ar trebui sa aibă o nuanță pozitivă orientate spre îmbunătățirea codului. Încearcă să relaționezi comentariile cu standardele de codare, specificații, constrângeri de performanță, securite, etc.
- Tratează cu respect și răbdare persoanele care au un nivel de cunoștințe mai scăzut decât al tău. Gândește-te cam cui ai vrea să fii și tu tratat de persoane care au mai multe cunoștințe decât tine și fă și tu la fel cu ceilalți.
- Adevărata autoritate vine din cunoștințe, nu din poziția ierarhică. Cunoștințele generează autoritate și autoritatea generează respect. Nu vei fi niciodată respectat dacă spui "Că așa zic eu!"
- Întâlnirile de Code Review nu sunt întâlniri pentru rezolvare problemelor.
- Pune întrebări în loc să faci afirmații. O afirmație este de obicei acuzatoare: "Nu ai urmat standardele de codare", chiar dacă nu este intenționat. Spunând "Care a fost motivarea din urma deciziilor pe care le-ai luat?" încerci să afli informații, nu să acuzi.
- Încearcă să eviți "DE CE"-urile. Deși este foarte dicifil de cele mai multe ori, evitarea întrebărilor de tip DE CE poate păstra o atmosfera degajată în echipă. O întrebare de tipul "De ce nu ai urmat standardele de codare in cazul asta?" Poate fi ușor reformulată spre "Care au fost motivele pentru care nu ai urmat standardele de codare?"
- Laudă persoanele atunci când codul lor e "ca la carte". Firea umană este construită să aibă nevoie de cuvinte de încurajare și laude, nu doar de critici. De cele mai multe ori un cuvând de încurajare are un efect mult mai bun decât 100 de critici.
- Coding Standards. Orice proces de Code Review trebuie să aibă la bază standardele de codare ale proiectului și/sau organizației. Coding Standard este ca un contract între dezvoltatori pentru respectarea calității și mentenabilității codului.
- Ține minte că mereu există mai mult decât o soluție pentru rezolvarea unei probleme. Chiar dacă dezvoltatorul a urmat altă soluție față de cea la care te-ai gândit tu, nu înseamnă că soluția este greșită.
- Nu trebuie să te grăbești atunci când faci Code Review, dar trebuie să fii cât mai prompt.
- Încearcă să nu depășești 200-400 de linii de cod per review session.
- Nu ești "Dumnezeul" programării. Nu uita că și tu ai fost în etapa în care acumulai cunoștinte și făceai foarte multe greșeli. Doar pentru că ești reviewer nu înseamnă că ai puteri depline asupra codului și a proiectului.
Security Code Review
Dacă aplicația necesită o atenție deosebită asupra părții de securitate, vă recomand www.owasp.org. Există și un document de Code Review foarte bun: https://www.owasp.org/images/2/2e/OWASP_Code_Review_Guide-V1_1.pdf.
Pentru determinarea severității problemelor găsite în timpul code review-ului vă recomandăm nivele de severitate de mai jos:
- Naming Conventions and Coding style = Low,
- Control Structures and Logical issues = Medium or High,
- Redundant Code = High,
- Performance Issues =High,
- Security Issues = High,
- Scalability Issues= High,
- Functional Issues =High,
- Error Handling = High,
- Reusability = Medium.
Reviewerii ar trebui să se concentreze pe nivelele High și Medium.
Checklist-uri pentru dezvoltatori și reviewer
Checklist-urile sunt o unealtă foarte bună atunci când vrei să nu omiți anumite aspecte și sunt foarte utile atât pentru dezvoltatori cât și pentru revieweri. Omisiunile sunt defectele cele mai greu de reparat - clar că nu ai cum să faci review la ceva ce nu există. Un checklist e cel mai simplu mod prin care poți combate asta.
Un alt concept foarte folositor este checklist-ul personal. O persoană face în medie cam aceleași 15-20 de greșeli. Pentru a preveni repetarea, încearcă să ții o listă cu problemele cele mai întâlnite. Aceasta te va ajuta și pe tine ca dezvoltator să îți însușești conceptele respective.
Checklist pentru Dezvoltatori
Description
|
Confirmed?
|
Codul se compilează
|
|
Codul a fost testat de mine și include unit teste
|
|
Codul include javadoc
|
|
Codul este ordonat (spațiere, greșeli de ortografie, nu există cod comentat, etc)
|
|
Am considerat utilizarea corectă a excepțiilor
|
|
Am utilizat adecvat logging-ul
|
|
Am eliminat importurile nefolosite
|
|
Am eliminat warning-urile raporte de Eclipse/Netbeans/IntelliJ
|
|
Am luat în considerare potențiale NPEs
|
|
Codul urmează standardele de codare
|
|
Am eliminat orice cod redundant sau clase care nu mai sunt folosite
|
|
Am eliminat orice hardcodări sau development-only code?
|
|
Perfomanța a fost luată în considerare?
|
|
Securitatea a fost luată în considerare?
|
|
Codul eliberează resursele? (HTTP connections, DB Connections, files, etc)?
|
|
Cazurile particulare au fost bine documentate? La fel orice workaround sau limitare
|
|
Codul poate fi înlocuit cu apeluri către componente externe resutilizabile?
|
|
Thread safety și posibile deadlocks
|
|
Checklist pentru Revieweri
Description
|
Confirmed?
|
Comentarile sunt comprehensibile și aduc valore mentenabilității codului
|
|
Comentariile nu sunt foarte multe și nici foarte amănunțite
|
|
Tipurile de date au fost generalizate acolo unde a fost posibil
|
|
Tipurile parametrizate au fost folosite corespunzător
|
|
Excepțiile au fost folosite corespunzător
|
|
Codul duplicat a fost eliminat
|
|
Frameworkurile au fost folosite corespunzător
|
|
Clasele de tip comand sunt concentrate pe un singur task
|
|
JSP-urile nu conțin logică de business
|
|
Unit test-ele există și sunt corecte
|
|
Sunt verificări pentru erorile comune
|
|
Potențiale probleme de multi-threading au fost eliminate pe cât posibil
|
|
Eventuale probeme de securitate au fost adresate
|
|
Performanța a fost luată în considerare
|
|
Funcționalitate se încadrează în design-ul/arhitectura curentă
|
|
Codul este unit testable
|
|
Codul nu folosește nejustificat elemente statice
|
|
Codul urmează standardele de codare
|
|
Logging-ul a fost folosit corespunzător
|
|
NPEs și AIOBs
|
|
Automatizarea procesului de Code Review
Pornind de la un document cu standarde de codare, o parte din procesul de Code Review poate fi automatizat. Asta nu înseamnă că rolul de Reviewer nu mai există. Tool-urile care automatizează Code Review-ul, vor elimina mare parte din problemele legate de styling, convenții de denumire, complexitate ciclomatică a claselor, metodelor, cod duplicat, cât de mult acperă unit testele codul scris, probleme minore de design, etc. Nu pot detecta însă probleme majore de design, arhitectură, funcționalitate care sunt specifice proiectului. Voi prezenta mai jos cele mai folosite tool-uri pentru Java.
PMD
Pagina oficială: http://pmd.sourceforge.net/.
Este un analizor de cod sursă. Detectează variabile neutilizate, probleme de styling, denumire, blocuri goale, complexitte ciclomatică, etc. Are integrare cu tool-urile populare de building: Maven și Ant, precum și cu cele mai populare IDE-uri: Eclipse, Netbeans.
Findbugs
Pagina oficială: http://findbugs.sourceforge.net/.
Folosește analiza statică a codului pentru identifiare bug-urilor din cod. La fel ca si PMD are integrare cu cele mai populare tool-uri de building si IDE-uri.
Checkstyle
Pagina oficială: http://checkstyle.sourceforge.net/
Este concentrat în principal pe styling, denumiri, cod duplicate, cod duplicat, etc. La fel ca și PMD are integrare cu cele mai populare tool-uri de building și IDE-uri.
Code Coverage
Pentru a verifica gradul de acoperile al codului cu unit teste cele mai populare tooluri sunt: Jacoco, Cobertura și Clover.
Sonar
Sonar este un tool care agregă datele prezentate de toolurile de mai sus și le afișează într-o interfață foarte ușor de folosit, oferind diverse rapoarte, grafice, evoluția codului de-a lungul timpului. Pe lângă toolurile prezentate mai sus, Sonar oferă și multe alte pluginuri care ajută la procesul de Code Review.
Sonar oferă integrare cu cele mai populare IDE-uri și tool-uri de building și oferă suport out-of-the-box pentru Java. Poate fi configurat și pentru alte limbaje de programare precum PHP, C++ sau .NET dar cu puțin mai mult efort.
Aveți în partea de jos a paginii o diagramă care prezintă arhitectura Sonar-ului. (Server se referă la serverul de Sonar).
Controlarea procesului de Code Review
Până acum am prezentat sfaturi despre ce anume se urmărește într-un proces de code review și cum se poate automatiza o parte din code review pentru a permite Reviewer-ului să se concentreze asupra lucrurilor importante și să nu își piardă timpul verificând cât de lungi sunt liniile de cod sau dacă există spații între operatori și operanzi.
Cum e cel mai bine să urmărim progresul: prin email, printr-un fișier excel, doar discutând, folosind tooluri specilizate? Deși răspunsul cel mai evident este folosirea tool-urilor specializate, fiecare echipă și proiect are particularitățile sale. De obicei, fiecare echipă își găsește propria mecanică care va avea diverse particularități:
- Poți avea o regulă ca nimeni nu dă commit în version control system până când un alt coleg nu face code review la cod.
- Fiecare se poate uita pe toate schimbările care vin în momentul în care face update din version control system.
- Există o persoană per zi/per spint/per iterație care face review pentru toți ceilalți.
- Formăm echipe de câte două persoane și fiecare face code review la celălalt.
- Avem o echipă dedicată de code review formată doar din persoane cu foarte multă experiență.
Dacă sunteți într-un mediu în care folosiți tool-uri de la Atlassian (JIRA, Confluence, Fisheye), alegerea cea mai evidentă este Crucible datorită integrării cu celelalte produse. În momentul de față cred că este cel mai bun tool pentru monitorizarea procesului de code review. Alte alegeri ar mai fi: Gerrit (dedicat proiectelor ce folosesc Git) sau ReviewBorad.