Katselmointi (Code review / Koodin katselmointi)

Mitä on koodin katselmointi?

Koodin katselmoinnilla tarkoitetaan lähdekoodin ajoittaista tarkistamista virheiden varalta.

Miksi?

Jos muistatte 25.9. luennon (legacy koodi), Cinian ukko kertoi kauhutarinoitaan legacyn kanssa kamppailusta. Miten oltaisiin vältytty nested ternary operaatioilta ja DIY tietotyypeiltä?

Vastaus on koodin katselmointi.

Koodin katselmoinnin tarkoituksena on antaa vertaisten käydä läpi ja arvioida tekemääsi koodia. Jonkun verran ohjelmoineena tiedän tunteen kun mikään ei toimi. On helppo sortua ottamaan oikotie onneen ja kirjoittaa vaan jotain mikä toimii. Ei hyvä. Koti/kouluprojektissa siitä ei ole niin haittaa, mutta "oikeassa" projektissa tuommoiset hackit kasvattavat nopeasti teknistä velkaa (technical debt).

Huono koodi ja bugit eivät ole ainoita asioita joihin code reviewt auttavat. Koodi voi olla samalla oikein kirjoitettua mutta huonoa. Katso: /r/programminghorror. Muita esimerkkejä ovat: * Koodi ei ole luettavaa * Koodi ei vastaa yrityksen X koodaus standardeja. * Koodi ei ole ylläpidettävää. * Arkkitehtuuri pielessä. * Ohjelmoijat eivät ole samalla sivulla siitä miten asioita tehdään

Ohjelmoinnissa on se kaunis piirre että yhteen ongelmaan on olemassa monta ratkaisua. Annan esimerkin.

Fizz buzz testi

Fizz buzz testi on pahamaineinen työhaastattelukysymys jossa tarkoituksena on tulostaa luvut 1 -> 100, mutta tulostaa numeron sijaan Fizz kun luku on 3 jaettava, Buzz kun luku on 5 jaettava ja Fizz Buzz kun molemmat pätee.

Kuulostaa helpolta.

Ensimmäinen esimerkki - Perus ratkaisu

for (var i=1; i < 101; i++){
    if (i % 15 == 0) console.log("FizzBuzz");
    else if (i % 3 == 0) console.log("Fizz");
    else if (i % 5 == 0) console.log("Buzz");
    else console.log(i);
}

Toinen esimerkki - Lyhyt ratkaisu

for(let i=0;i<100;)console.log((++i%3?'':'fizz')+(i%5?'':'buzz')||i)

Kolmas esimerkki - Enterprise ratkaisu

https://github.com/EnterpriseQualityCoding/FizzBuzzEnterpriseEdition

Mitä voimme ottaa tästä irti?

Ensimmäinen ratkaisu on helposti luettavissa. Ohjelmoinnin perusteet omaava henkilö voi seurata koodia helposti. Jatkoon.

Toinen ratkaisu on lyhin mahdollinen ratkaisu minkä löysin. Se että sen toimintaperiaatteen selittämiseen on omistettu pitkähkö blogikirjoitus kertoo siitä että se ei ole helposti luettavissa. Ei jatkoon.

Kolmas ratkaisu on summonattu suoraan ohjelmointihelvetin syvimmästä perukasta. Tähti repoon ja ei todellakaan jatkoon.

Kuvitelkaa kokonainen ohjelma joka on kirjoitettu käyttäen toisen ratkaisun taktiikkaa jossa kaikki tiivistetään mahdollisimman pieneen tilaan. Ohjelman kirjoittanut ohjelmoija on varmasti todella tyytyväinen itseensä. Onhan mahdollisimman elegantin ratkaisun löytäminen tyydyttävää. Vaikka omaankin rajatun kokemuksen ohjelmointialasta uskaltaisin väittää että osa ongelmista syntyy juurikin ohjelmoijien tarpeesta luoda mahdollisimman (ainakin omasta mielestä) elegantteja ratkaisuja koodin luettavuuden kustannuksella.

Elegantissa ja ytimekkäässä (concise) koodissa ei ole ongelmaa jos koko kehitystiimi on samalla sivulla sen käytöstä. Siksi pidetään koodi katselmointeja joissa kakkos esimerkin kirjoittanut ohjelmoija kertoo miksi teki ja muut kertovat miksi ei tehdä.

Miten

Fagan tarkastus

Fagan tarkastus on Michael Faganin 70-luvulla kehittämä tarkastusprosessi. Kuten alla copy pasteemassani tekstissä näkyy, prosessi on erittäin työläs ja sen takia kritisoitu. 

Tarkastusprosessin vaiheet ovat: suunnittelu, yleiskatsaus, valmistautuminen, tarkastuspalaveri, virheiden korjaus ja seuranta. Valmistautuminen, tarkastuspalaveri ja virheiden korjaus -vaiheita voidaan iteroida.

Suunnittelu: tarkastuksen puheenjohtaja suunnittelee tarkastuksen.
Yleiskatsaus: tekijä esittelee tuotteen taustoja.
Valmistautuminen: jokainen tarkastaja tutkii tuotteen mahdollisten virheiden varalta.
Tarkastuspalaveri: palaverin aikana sihteeri lukee tuotteen läpi kohta kohdalta ja tarkastajat ilmoittavat jokaiseen kohtaan löytämänsä virheet.
Virheiden korjaus: tekijä korjaa virheet tarkastuspalaverissa saadun palautteen mukaan.
Seuranta: tekijän tekemät muutokset tarkastetaan jotta virheet on saatu varmasti korjattua.
Puheenjohtaja lopettaa prosessin kun ennalta määritellyt lopetuskriteerit täyttyvät.

Lähde: https://fi.wikipedia.org/wiki/Ohjelmistojen_tarkastus

Muutospohjainen tarkistus

Esimerkki toiminnasta:

  1. Kehittäjä kehittää toimintoa X omassa branchissa
  2. Kehittäjä muuttaa olemassaolevia rajapintoja
  3. Kehittäjä tekee uutta
  4. Kehittäjä lähettää merge requestin masteriin
  5. Muu ryhmä lukee läpi merge requestin muutokset ja hyväksyy/hylkää