Koodikatselmointi

[samushka13]

To 29.4. n. klo 00:00

Yleiset kommentit ja kehut

• Koodissa on selkeät rakenteet ja nimentätavat. Koodia lukemalla on hyvin helppo ymmärtää, mitä missäkin tapahtuu. Selkeä dokumentaatio avustaa lisäksi mallikkaasti.

• Testit on kauttaaltaan toteutettu hyvin ja ne testaavat olennaisia asioita. Testikattavuus oli jo tässä vaiheessa varsin korkea.

• Kokonaisuudessaan todella hyvin noudatettu kurssilla esitettyjä koodin laatuvaatimuksia.

• Erityiskehut savolaisten vaikeusasteiden soveltamisesta pelissä :)

Kehitysehdotukset (lähinnä pilkunviilausta)

• Luokkien suorista riippuvuuksista voisi yrittää päästä vieläkin tehokkaammin irti. Esimerkiksi luokassa Gameloop olevat Menu- ja VisualBoard-oliot voisi tallentaa muuttujiin ja sitten injektoida näitä muuttujia aina tarpeen tullen. Tämä auttaa esimerkiksi tilanteessa, jossa injektoitu luokka saa lisää attribuutteja, sillä attribuutteja tarvitsee tällöin lisätä vain yhteen paikkaan.

• Luokan Gameloop metodin _handle_events voisi kenties jakaa useampaan osaan (tyyliin _handle_victory, _handle_quit…). Tämä voisi auttaa etenkin tilanteessa, jossa eventtien määrä kasvaisi nykyisestä.

• Pelin vaikeustasot ovat nyt koodissa numeroina, mutta sen sijaan voisi harkita niiden tallentamista vakioarvoon. Siten olisi helpompaa koodia lukiessa muistaa, mitä esimerkiksi vaikeustaso 1 tarkoittaa, etenkin, jos vaikeustasojen määrä kasvaisi tulevaisuudessa.

• Testien osalta voisi harkita assertTruen/assertFalsen korvaamista assertEqualsilla/assertNotEqualsilla, sillä assertin feilatessa jälkimmäiset tarjoavat virheraportissa yksityiskohtaisemman tiedon siitä, mikä meni pieleen. Lisäksi assertEquals on ns. null-safe, vaikkakaan tällä seikalla ei liene tässä tapauksessa kovin suurta merkitystä.

• Metodeissa usein esiintyviin totuusarvojen palauttamisiin voisi soveltaa useammin metodia bool(), kenties metodia any() yhdistellen, mikäli ehtotarkistuksia on useita. Esimerkiksi luokan GameLogic metodin can_add_value voisi toteuttaa siten, että metodi palauttaa metodin any() tuloksen sisältämästään ehtotarkistusten listasta (ks. esimerkki alla). Etu tässä on siis se, että metodille tarvitsee tällöin määritellä vain yksi return lause (nykyisellään niitä on esimerkiksi edellä mainitussa metodissa viisi) eikä arvoja True/False tarvitse erikseen kirjoittaa auki. Lienee kuitenkin makuasia, kumpaa tapaa pitää helpompana lukea.


Yhden totuusarvon tarkastus (palauttaa True, jos ehto täsmää, False, jos ei):

return bool(ehto)

Useamman tarkastus (palauttaa True, jos mikä tahansa ehto täsmää, False, jos mikään ei):

return any([
    bool(ehto1),
    bool(ehto2),
    bool(ehto3),
    bool(ehto4),
])

Toivottavasti näistä kommenteista on jotain hyötyä. Tsemiä ja mukavaa kevään jatkoa!

[Aikamoine]

Iso kiitos! Ihan esimerkillinen katselmointi. Tästä sai paljon kehitysideoita, ja hyvän mielen :)