Legat de ceea ce am observat eu la primul meu code review:
1.) Replace Nested Conditional with Guard Clauses
Daca ai if in if pe mai multe nivele de nesting asta se poate refactoriza in a “flat” list of conditionals.
2.) Daca ai un if cu multe conditii, tot sirul de conditii se pot pune intr-o metoda cu nume sugestiv si sa se foloseasca in if aceea metoda astfel incat sa nu trebuiasca sa parcurgi toate conditiile ca sa iti dai seama cand se executa if-ul respectiv ci sa iti dai seama din numele metodei.
3.) Daca ai un if cu un bloc de instructiuni, ai putea sa extragi acel bloc de instructiuni intr-o metoda cu un nume sugestiv astfel incat sa intelegi codul fara sa parcurgi secvential toate instructiunile din bloc, ci doar sa te uiti la o interfata high level, o semnatura de metoda.
Cand scrii cod te concentrezi sa rezolvi problema in timp cat mai scurt si mai putin pe cat de usor de inteles este acel cod de catre altcineva sau cat de mentenabil e.
O lista completa de code smells si solutii de refactoring pentru ele, foarte bine structurata, pe care puteti sa le luati in considerare cand scrieti code sau faceti code review altcuiva:
Unii pot considera aceste principii exagerate, altii prea simple sa fie aplicate, dar ele sunt vechi, rezultatul experientei marilor software craftsmans ca Robert C. Martin si Martin Fowler, prezentate si in Clean Code, cartea si seria video.
Aceste principii alaturi de altele ca si unit testing asigura calitatea si mentenabilitatea produsului software.
Nu ma pronunt in privinta modului de executie corect al unui code review (am refuzat de multe ori sa fac asta), dar mi-a sarit in cap chestia de mai sus. Si e o prostie enorma, cel putin asta cred eu. Cand scriu cod cel mai important lucru este sa fie lizibil. De mine peste 1-2-3 saptamani, sau dupa o dimineata in care nevasta a urlat la mine ca nu am luat paine, sau dupa 3 beri, sau de alti colegi. Iar problema se presupune ca ai rezolvat-o inainte sa incepi sa scrii cod, de aia avem foi de hartie, pixuri si creier. Daca intri in problema direct in cod, o sa orbecai fara tinta sperand ca o sa-ti iasa. Si daca esti bun, o sa-ti iasa. Ochii din cap.
Da ai dreptate, sa ai solutia deja in minte cand ai inceput sa scrii cod asta te ajuta foarte mult dar nu iti va garanta ca vei scrie un cod lizibil, pentru asta sunt un set de reguli pe care sa le urmezi, daca nu le folosesti vei scrie tot cod nelizibil.
Problema de lizibilitate a codului(daca nu e usor de inteles si de modificat) e un code smell care de obicei ascunde alte code smells, e bine sa poti recunoaste aceste code smells si sa cunosti solutii de refactoring pentru ele. IDE-ul poate contine un tool sa detecteze aceste code smells, code review poata fii alta solutie pentru detectarea acestor code smells unele pe care IDE-ul nu le poate detecta, legat de faptul daca e prostie sau nu code review-ul, nu ma pronunt, practica asta exista pe proiect, dar eu o vad ca pe modalitate de a te perfectiona chiar daca iti face altcineva code review sau daca faci tu code review, motivul pentru care am observat ca nu se face code review unde nu se face e lipsa de timp si graba.
Nesting-ul aiurea la if-uri se vede de la 1km distanta si nu e bine. In rest tine de standarde pe proiect, se pot face chiar snippet-uri pentru anumite cazuri pe care toti sa le foloseasca.
Deşi sunt clar adeptul guard clauses, există totuşi (ca deobicei) şi excepţii de la (orice) regulă de aur:
în cazul unui algoritm care foloseşte extrem de intensiv acea funcţie (cu mai multe if-uri), folosirea guard clauses-urilor s-ar putea să te penalizeze la capitolul performanţă, în contrast cu if-uri imbricate
dat fiind faptul că ai multe if-uri înşirate la începutul funcţiei, corpul propriu-zis al funcţiei tale este practic locat într-o ultimă ramură else a unei ierarhii invizibile de if-uri imbricate
astfel există o şansă mare ca predicţia să aleagă mai des conţinutul ramurilor then a if-urilor cu rol de guard clauses (tratare cazuri de excepţie, care ar trebui să se execute/aibe loc rar) şi o şansă mai mică să se ajungă cât rapid/eficient la else-ul cu conţinutul productiv (care ar trebui să fie ramura executată cel mai des/probabil şi pe care o doreşti să fie aleasă întotdeauna de către predicţie)
în constrast: într-o ierarhie de if-uri imbricate, în care distribui conţinutul productiv cât mai sus în ierarhia de if-uri, în funcţie de probabilitatea de a fi executat mai des, pe care o consideri tu (şi care este mai greu de intuit pentru compilator/CPU) s-ar putea să ajuţi predicţia să aleagă macazul corect
fireşte, vorbesc aici de un caz de excepţie, care rar va fi întâmpinat şi care în majoritatea cazurilor va fi optimizat suficient de bine de către compilatoarele tot mai dăştepte, care fac o analiză statică şi dinamică a codului, pentru a însira instrucţiunile încât să fie “nimerite” cât mai des de predicţia CPU-ului
totuşi - în cazuri rare - penalităţi de performanţă pot apărea şi în ciuda optimizărilor, iar pentru cine ar crede că ar fi relevant doar pentru limbaje compilate/native, am găsit un exemplu de contraargument precum următoarea intrebare faimoasă de pe StackOverflow