Koodikatselmointi

[elgecaro]

Ohjelma on ladattu 28.4 klo 10:43

Yleinen palaute

• Pakkausrakenne on selkeä ja pakkaukset muodostavat hyvät kokonaisuudet (käyttöliittymä, tietojen talletukseen liittyvät luokat ym.).
• Luokkien nimet ja niiden muodostavat kokonaisuudet hyvät ja suhteellisen selkeät.
• Yleisesti ottaen myös luokkien metodit, attribuutit, parametrit ja muuttujat ovat hyvin ja kuvaavasti nimetettyjä.
• Koodi on selkeä, metodit ja luokat hyvän pituisia.

Parannusehdotuksia

Main

• Luokassa käytetään suoraan DAO:ja, esim. level = levelDao.findOne(levelName)) sekä ArrayList<String> levelNames = levelDao.listAll() näitä voisi mieluummin käyttää sovelluslogiikan kautta.
• Myös esim. leftPlayer.addTie-metodi voisi tehdä sovelluslogiikan kautta.
• Mainissa on olemassa hbox ja vbox-nimiset komponentit, niiden nimet voisivat ehkä olla vähän selkeämmät, vaikkakin aika helposti näkee mitä ne sisältävät.

Game

• Game:issa oleva metodin ammunitionXwithDrag sekä ammunitionYwithDrag muuttujien nimet (ainakin minulle) vähän epäselviä mitä ne ovat ja mihin tarkoitukseen:
  • a, v, g ja p sekä up1, up2 ja up3 (Mutta näitä käytetään kuitenkin vain noissa metodeissa, eli ei kovin suuri juttu.)

PlayerDao ja LevelDao

• Tekisin ehkä näille DAO:ille omat rajapinnat, niin DAO:t olisivat piilotetetut sovelluslogiikalta.

Testit

• Testit vaikuttavat toimivan hyvin, mutta kaikille eri luokille mitä testataan pitäisi olla omat testiluokat, eli esim. playerDaoTest, GameTest ym.
• Kun testit tehdään sen jälkeen että peliä on pelattu ja tietokantaan on lisätty tietoja, testi databaseReturnsWinners() on rikki ja ilmoittaa esimerkiksi: java.lang.AssertionError: expected:<2> but was:<5> Eli pelaajia yhteensä on enemmän kuin alussa. Testeille pitäisi siksi ehkä olla oma testi-tietokanta, niin tätä ongelmaa vältetään.

Muuta

• Vaihtaisin ehkä juuripakkauksen nimen pelin nimeksi, kun nyt on esim. otmharjoitustyo.database.
• Voisit mahdollisesti myös lisätä konfiguraation tietokannnalle, niin sovelluksen käyttäjä voisi helposti määrittää tietokantatiedoston nimen.
• Voisit myös luoda metodin joka luo tietokantatiedoston ja sen tarvittavat tiedot jos tietokanta ei entuudestaan ole olemassa.
• Tässä versiossa ei tehdä mitään Exceptionille (esim. virheilmoitus), mutta sitä varmaan vielä muokataan.

Yleisesti ottaen kuitenkin hyvin toteutettu peli ja koodi, huomaa että olet laittanut paljon aikaa tähän!

[hannuee]

Kiitos hyvästä katselmoinnista! 👍 Hyviä huomioita joista muutamaa en ollut tullut ajatelleeksi itse ollenkaan. Erityisesti tuo tietokannan tekeminen on hyvä idea joka ei omaan mieleeni ollut tullut, näin saan myös releasesta siistimmän kun releaseen tarvitsee laittaa vain .jar, ei ollenkaan .db:tä. (edit: tai no pelikentät siellä .db:ssä pitää kyllä olla valmiina, mutta ainakin Players:in osalta noin voisi tehdä)