Ir pagājis jau krietns laiciņš kopš veicu code-review laiku pa laikam. Sākumā bija doma pierakstīt katru atrasto lietu un apskatīt to sīkāk, bet ar laiku entuziasms noplaka nedaudz, bet nu gala rezultātā esmu tomēr apkopojis septiņas visizplatītākās kļūdas vai nianses, kas atkārtojas no code-review uz code-review.
Lielākā daļa ir neatkarīgi no cilvēkiem, projektiem un pat tehnoloģijām, kuras pielieto kādā konkrētajā projektā vai sistēmā. Šeit ir saraksts ir izplatītākajām issues, kas tik atrastas kārtējā code review procesā:
- Naming convention
- Unused namespaces
- this qualifier
- StyleCop un FxCop violations
- Complex conditional statements
- File name does not match type name
- Unused local scope variables
Naming convention. Šis ir viens no visizplatītākajiem pārkāpumiem vai issues, kas tiek atrasts kārtējā izejas koda pārskata procedūrā. Lai arī kāda būtu konkrētā projekta norunātā naming konvencija, tā vienmēr tiek pārkāpta. Vai nu tie ir nepareizi izvēlēti klases nosaukumi (piemēram, ir projekti, kas mēģina izvairīties no “*Manager.cs” vai “*Manager.vb” failiem, kas savukārt var norādīt uz SRP principa pārkāpumu).
Nav nozīmes, kurā līmenī ir pārkāpums: vai tas tiek darīts kodā, vai HTML’ā vai pat datubāzē. Jebkura pārkāpuma detektēšanai ir nepieciešams ieviest kādus automātiskos rīkus un paņēmienus. Pāris idejas:
- Koda re-formatēšanas operācija notiek katru nakti – būvējuma laikā. Tādā veidā izstrādātājiem nav iespējas veikt tīšu/netīšu nosacījumu un norunu pārkāpumu. Tās visas tiek “salabotas” būvējuma laikā.
- Izmantot kādu koda analīzes rīku, kas meklē naming convetion pārkāpumus (piemēram, FxCop var pastāstīt, ka projektā nav pieņemts, ka metodes nosaukumus satur apakšvītru simbolu “_”).
- Interesants piegājiens bija vienam no maniem kolēģiem. Datu bāzes objektu naming convention () pārbaude notiek ar speciālu šim nolūkam izveidot vaicājumu, kas apstaigā SQL servera sistēmas viewus un tabulas un apskatās vai, piemēram, visi primary atslēgas objekti sākas ar “Pk_” simbolu virkni. Ja kāds no noteikumiem tiek pārkāpts šo vaicājumi ātri var iedarbināt būvējuma laikā un jebkurš pārkāpums tiks fiksēts ar būvējumu, kas izgāžas.
Unused namespaces. It kā jau nekas traģisks un kādu nenormālo impaktu tas nerada, bet tomēr šķiet ka te kaut kas nav īsti pareizi un pārskatāmi.
this qualifier. Arī šis pārkāpums īsti neko ļaunu nenodara, bet ļoti bieži kodā ir redzams, ka “this.” tiek over-pielietots. tas nozīmē, ka jebkurā vietā klases pirmkodā, kur notiek griešanās pie pašreizējās klases vērtībām, mainīgajiem vai kādiem citiem klases slotiem, izsaukums tiek uzsākts ar “this.”. Viss ir forši un uzreiz pēc punkta “.” parādās IntelliSense ar visām iespējamajām kombinācijām un izvēlēm pašreizējās klases kontekstā, bet IntelliSense izvēlnei var izmantot arī citus paņēmienus.
StyleCop un FxCop violations. Tas protams ir atkarīgs no projekta, bet ir projekti, kuros mēģina ievērot gan StyleCop, gan arī Code Analysis (a.k.a FxCop) ietikumus un nosacījumus. Ļoti bieži saskaros ar problēmu, ka code-review laikā daudzi no ieteikumiem tiek pārkāpti. Izrādās, ka šie noteikumi ir tikai uz papīra un to ievērošana ir tikai formāls pasākums. Lai tā nebūtu, ieteicams šos noteikumus “ieforsēt” jeb padarīt neiespējamu tos pārkāpt. Viens no klasiskajiem variantiem ir ieslēgt TFS checkin politikas, ja tiek izmantots Team Foundation Server platforma, kur uzglabāt izejas kodu sistēmai vai projektam. Otrs variants ir vēl visas Code Analysis pārbaudes pārslēgt uz “Error” režīmu, kas kļūst par būvējuma grāvējiem situācijās, kad kāds ir pārkāpis šos noteikumus. Ļoti palīdz :)
Complex conditional statements. Šis bija viens no Clean Code Cheat Sheet norādījumiem, kas parāda, ka visticamāk šis nosacījums, kas ietverts “if” statement iekavās nav pārāk saprotams un uztverams, kur nu vēl modificējams vai refaktorējams bez kļūdām.
Šāda veida issues viens no variantiem, ka varētu noķert ar koda metrikām, bet visātrāk tos ievērot un salabot laikam jau ir manuālajos code-review procesos.
if (parameter.Count > 0 && Client.MustApply &&
(Client.Gender == HumanGender.Male || Client.Age > 50) &&
((selectedProgram.Id == 45 && chosedRisk.Id == 1945) || IsFreeProgram.Checked))
{
// do some magic...
}
File name does not match type name. Arī nav baigās problēmas dēļ šī, bet ļoti izplatība problēma. Zināmas grūtības varētu rasties, kad sistēmas izejas kods tiek pētīts un analizēts bez VS object browser vai kādu citu sakarīgu programmu, kas skatās uz loģisko sistēmas saturu nevis fizisko. Piemēram, bieži rodas situācijas, kad ražošanas vides serverī, kuram pieeja iedota tikai uz 30 minūtēm, jo redziet kāds sistēmas administrators iedomājies, ka vairāk laika programmatūras piegādātājiem reāli kritiskā situācijās problēmas risinājumam vairāk laika nebūs nepieciešams, ir jāpārlūko failu sistēmas saturs un tikai vadoties pēc failu nosaukumiem ir jāsaprot kurā klasē ir jāveic izmaiņas. Pēc tam var izrādīties, ka klase izejas kodā nav tāda pati, kāds bijis faila nosaukums, kas bijis cēlos kādai problēmai.
Šīs problēmas risinājums pieejams vienā no VS papildinājumiem – Resharper:
Unused local scope variables. Šis arī neko ļaunu pārāk nevar nodarīt klasiskajās situācijās, kad klase satur pa kādam liekam mainīgajam. Problēmas varētu sākties situācijās, kad klase satur kādu sen nelietotu mainīgo, kas degradē objekta veidošanas vai inicializēšanas procesam nepieciešamo laika sprīdi, ja izrādās, ka šis “nelietotais” mainīgais ir palicis pāri no kāda code-review/code-refactoring sesijas un aizņem patiešām ievērojami laika sprīdi, jo iespējams, ka inicializē vai izmanto kādu sen aizmirstu time-consuming operāciju.
Šādā veidā mēs uz ražošanas vides atklājām vienu interesantu memory-leak problēmu.
Arī par šādam lietām vienu no populārākajiem VS paplašinājumiem – Resharper – var iemācīt bļaustīties pamatīgāk nekā pēc noklusējuma uzstādījumiem tas drīkst darīt.
Happy coding!
Cerams, ka noderēs!