Det är svårt att granska kod. Men nödvändigt om man vill ha en långsiktigt underhållsbar kod. Jag tror att det är precis detta man ska tänka på när man granskar kod: kommer denna kod vara lätt att utöka, ändra på och felsöka, etc.
Jag har inte bestämt mig om parprogrammering eller kodgranskning är bäst. Båda tillsammans är naturligtvis bäst, men antagligen inte tidseffektivt. Om man har svårt att få till kodgranskningen i sin grupp bör man nog gå över till parprogrammering. Det är ju kontinuerlig kodgranskning, och kontinuerliga aktiviteter är ju trenden inom mjukvaruutveckling.
Här är i alla fall några områden som jag tycker man bör titta på. Denna lista kan bli hur lång som helst, men jag tror det är bättre att fokusera på ett fåtal viktiga saker Att koden fungerar för det huvudsakliga användningsfallet kollar inte jag. Det får man ta för givet. Annars har man problem som inte den bästa kodgranskning kan lösa.
- Implementation matchar dokumentation
- Klass & metod access - så strikt som möjlig
- Felhantering
- Exceptions
- Loggning
- Gränsvärdesvillkor (tex tom lista)
- Om equals överrids, då måste hashCode överridas (och vice versa)
- Tester: enhets och komponent och/eller integration om tillämpligt
- Storlek på klasser & metoder
- DRY - Don't Repeat Yourself
Implementation matchar dokumentation
Bättre ingen dokumentation än felaktig eller missvisande dokumentation.
Klass & metod access - så strikt som möjlig
Många verkar knappt känna till, än mindre använda default scope, aka package-private. Detta är det rätta scopet för implementationsmetoder och även klasser som inte kan vara privata men som inte bör läcka ut.
Felhantering
Programmeraren tänker ofta på fallet när allt går bra. Och ofta är felhanteringen ospecad. Men vad händer t.ex. om man får ett oväntat svar från ett annat system? Hanter koden det? Loggar den? Stängs resurser?Exceptions
Exceptions är kraftfulla rätt använt. De ger möjlighet till alternativ returpunkt och returtyp från en metod. Betänkt att det är en icke-lokal GOTO, och kan därmed göra det mer komplicerat. Många verkar skygga eller okunniga för att skapa egna Exception klasser. Gör gärna det. Med trevliga bekväma konstruktorer. Behöver inte bara vara String message och Throwable e utan kan innehålla mer data om felet som kan exponeras i en getter för klienter längre ner i call stacken.
Loggning
Lagom mycket, vid rätt tillfälle och på rätt nivå. Skriv gärna en hjälpmetod för loggning om du loggar liknande saker vid flera tillfällen. En toString() på klasser av värde typ minskar mängden kod för att logga enormt:
log.info("Did something with: " + this);
Och begå inte dödssynden, svälja ett exception stacktrace i din loggning:
catch(SomeException e) {
log.warn("something happend");
}
Så här ska det vara:
catch(SomeException e) {
log.warn("something happend", e);
}
Om det är värt att logga överhuvudtaget, så är det värt att logga även stacktracet.
Gränsvärdesvillkor (tex tom lista)
Hanterar koden null, tomma listor och andra gränsvärden på ett bra sätt. Kasta hellre IllegalArgumentException istället för att tyst svälja nåt som blir ett svårdebuggat problem långt senare i exekveringen.
Om equals överrids, då måste hashCode överridas (och vice versa)
Annars kan det bli jobbigt. Tro mig. Använd gärna automatgenerering i IDE:t. Då får man båda på en gång och konsistens till på köpet.
Tester: enhets och komponent och/eller integration om tillämpligt
Det finns väl tester för koden? Ju mer komplicerad kod, desto mer enhetstester. Tester fångar ofta fel som man aldrig tänkt på. Bättre med ett idiottest som "aldrig kan faila" än inga tester alls.
Storlek på klasser & metoder
Detta bör visserligen fångas i CI av ett verktyg för automatisk statisk kodanalys. Men ibland är långt under max-värdet det lämpliga.
DRY - Don't Repeat Yourself
Den underliggande principen för det mesta!
