Koodikatselmointi

[sallasal]

Projekti ladattiin GitHubista .zip-tiedostona sunnuntaina 6.12. noin klo 15.

Alla koodikatselmoinnissa heränneitä ajatuksia projektista. Kaiken kaikkiaan projekti on jo todella pitkällä, sovellus toimii vaatimusmäärittelyn mukaisesti ja koodi on useimmissa kohdissa erittäin selkeää luettavaa. Eli enää viilauksista kyse tässä. Pelilogiikka selvästi toimii, pelailin monta kierrosta.

Ohjelman suorittaminen

• Koodin lataaminen GitHubista onnistui hyvin.
• Koodin testit ajautuivat hyvin.
• Javadoc ja testikattavuusraportti ajautuivat hyvin.
• Ohjelma ajautui hyvin NetBeansin kautta eikä kaatunut erilaisilla syötteillä tms. Ohjelman suorittaminen oli selkeää ja peli toimi hyvin!
• Jos aikaa jää, laittaisin vielä napin “Pelaa uudelleen”, koska hävisin pelin ja halusin pelata helposti uudelleen.

Luettavuus

• Pakkausten, luokkien ja metodien nimet ovat konventioiden mukaiset ja yksiselitteiset.
• Metodit ovat selkeästi nimettyjä ja on selvää, mitä ne tekevät.
• JavaDocit siellä, missä ne ovat jo käytössä (kuten DAO-luokissa ja GameServicessä) ovat selkeät ja auttavat hyvin ymmärtämään ohjelmaluokkien suhteita toisiinsa.
• Käyttöliittymäkoodien pitkät metodit olivat vähän vaikeaselkoisia, ne saattaisivat hyötyä lisäkommentoinnista? En kyllä ole varma, mikä on kurssin linja käyttöliittymätiedostojen kommentoinnissa ylipäätään.
• Luettavuutta helpottaisi jatkossa vielä lisää, kun koodista poistettaisiin turhat ja/tai automaattisesti generoidut kommentit. Esim. -- automaattiset licence headerit vielä sekoittuvat tarpeellisen JavaDocin kanssa -- muun muassa MainGamessa on paljon poiskommentoitua koodia -- testeissä on automaattisesti generoitu testitesti ja muuta ylimääräistä vielä

Rakenne: ohjelma

• Rakenne on valtaosin hyvin selkeä, ja tiedostonkäsittely, pääohjelma ja eri luokat on jaettu pakkauksiin loogisesti.
• Kaikki tulostaminen tapahtuu selkeästi käyttöliittymässä, ja tiedostonkäsittely on selkeästi eriytetty.
• Ehkä miettisin vielä ennen kaikkea luokkien GameService, PlayerCommunication ja VenttiUI välistä työnjakoa, koska tavoitteena on, että kaikki pelimekaniikka ja sovelluslogiikka toteutettaisiin domain-pakkauksessa (siis käyttöliittymän ulkopuolella). -- Käytännössä miettisin joidenkin luokkamuuttujien (ainakin gameDeck, playerHand, computerHand) siirtoa VenttiUI:sta GameServiceen. Sieltä sitten välittäisin ehkä koko GameServicen muuttujana eteenpäin (sen sijaan että välittäisin yksittäisin muuttujia, kuten nyt VenttiUI välittää PlayerCommunicationille). Näin saisi käyttöön kaikki GameServicen sisäiset muuttujat, kun luokkien metoditkin siirtäisi samalla GameServiceen julkisiksi. Näin käyttöliittymästä voitaisiin kutsua aina GameServicen yhtä metodia sen sijaan, että laskentaa tai if-lauseita tarvitsisi tehdä käyttöliittymän puolella. Jonkin verran puuhaa tässä tietysti on, mutta metodit ovat jo olemassa kaikki, eli siirtelystä kyse. -- Säästäisin joka tapauksessa kaikki luokat, vaikka tuon siirron päättäisikin tehdä, koska niillä on selvästi myös omat tehtävänsä kullakin.

Rakenne: testit

• Fake-luokkaa on hyödynnetty testaamisessa tosi hyvin, ja muutenkin testiluokat on luotu kattavasti vastaamaan ohjelman luokkia.
• Testejä selkiyttäisi vielä entisestään, jos yksi testi vastaisi aina yhden asian testaamisesta. Käytännössä siis siten, että aina yhdessä testimetodissa olisi vain yksi testaava assert-rivi. Näin olikin käytännössä kaikissa muissa testiluokissa paitsi FileGameResultDaoTestissä.

[marykristina4]

Kiitos selkeästä ja yksityiskohtaisesta palautteesta! Olen tuon gameDeckin jo siirtänyt tuonne gameServicen puolella ja hand:ien suhteen olen pohtinut mikä olisi parasta. Teen varmaankin muut vaadittavat asiat ensin ja katson sitten jääkö tuohon vielä aikaa. Samaa mieltä olen siitä,että Uusi peli -toiminnallisuus pitäisi olla, koska peli on niin lyhyt. Sen tekeminen on siis ainakin korkeammalla prioriteetilla kuten myös poiskommentoitujen rivien siivoaminen, testien läpikäynti/siivoaminen jne.

Käyttöliittymälle en ole suonut sen enempää aikaa kun toiminnallisuuden rakentamiseen mennyt aika, ja varmistaminen että käyttöliittymä toimii oikein ja ohjaa painamaan toiminnallisuuden kannalta oikeita nappeja. Olen ymmärtänyt että tällä kurssilla käyttöliittymä ei ole niin merkityksellinen koodin laadun kannalta - tämä olisi toki hyvä vielä tarkistaa ohjaajilta. Ja lisäksi, jos davadociin sitten kuitenkin sisällyttäisi käyttöliittymän, voisi myös siinä dokumentoinnissa käyttöliittymän toimintaa vielä kuvata. Kiitos näistä huomioista!

Palautteesi toimii tosi hyvänä tarkistuslistana tällä viikolla tehtäville asioille, jotta saan sovellukseni kuntoon loppupalautukseen, kiitos!