Koodikatselmointi #2
Description
Projekti ladattu 18. kesäkuuta 2016, 20:11:58
Yleistä
Ohjelma toimi ainakin omalla koneellani hyvin ja nopeasti. Ainoa isompi bugi oli että jos jompi kumpi pelaaja ei saa yhtään maalia, tuloksia ei tallenneta koska GUI:n setResults-metodia ei kutsuta.
Mielestäni luokat sovelsivat valmiita rajapintoja kekseliäästi. Itse luotu Updatable-rajapinta on myös mielestäni hyvä idea. GUI ja logiikka päivitetään erikseen ja ennalta säädetyin väliajoin (Timer) mikä on myös hyvä.
Koodi oli tosi selkeää, luokat ja metodit eivät olleet liian pitkiä. Olen itse swingin kanssa niin noviisi etten oikein osaa sanoa siitä muuta, kuin että toteutus oli parempaa kuin omassa koodissani ja ikkunoiden hallinnointi toimii paremmin kuin minulla. Minulle ehdotettiin pajassa jonkinlaista erillistä luokkaa joka pitäisi kirjaa siitä mitkä ikkunat ovat auki ja asettaisi niitä näkyviksi tarpeen mukaan, inaktivoisi JButtoneita jne.
Nyt ohjelmassa on mahdollista esimerkiksi avata monta SelectStandings-ikkunaa tai vaihtaa Standingsia kesken pelin. (Kun tein näin, tulokset eivät vaikuttaneet tallentuvan kumpaankaan tuloslistaan, mikä on varmaan ihan hyvä...) Ehkä olisi yksinkertaisinta jos pelin ajaksi inaktivoitaisiin tulostilaston vaihtaminen,
Helpoin tapa minkä itse keksin tämän säätelemiseksi on (inaktivoimalla Select Standings -nappi) estää käyttäjää avaamasta Select Standings - ikkunaa, jos yksi sellainen on jo auki tai jos peli on kesken.
GUI.java
Huomasin että jos toinen pelaaja tekee pelissä nolla maalia, tulokset eivät näy tuloslistassa. Siis esim. 0-12 -tulos ei tallennu.
Syy lienee GUI-luokan setResults-metodissa rivillä 239: Logic-luokan setScores-metodia kutsutaan vain, jos molempien pelaajien pisteet ovat enemmän kuin nolla. Kokeilin muuttaa tässä ">"-operaattorit ">="-operaattoreiksi, ja tämän jälkeen myös sellaiset pelit tallentuivat joissa jompi kumpi pelaaja jää nollille.
Referee.java
Mietin tarvitseeko Referee-luokka erillistä Timerin kuuntelijaa, luokkahan voi myös itse implementoida ActionListenerin, jolloin luokassa toteutetaan @override-metodina actionPerformed. Tällöin esimerkiksi Refereen startGame-metodissa ajastin voitaisiin alustaa seuraavasti:
this.timer = new Timer(1000, this);
(Tällaista olion itsensä antamista omalle metodilleen parametrina ei voi muuten tehdä konstruktorissa, vaan instanssin pitää olla jo luotu.)
Tällöin ei ehkä tarvitsisi myöskään luoda kahden pituista ArrayListiä Updatable-objekteista TimerListenerille annettavaksi, vaan overridattu actionPerformed-metodi voisi käyttää guita ja logiikkaa, jotka ovat Refereen oliomuuttujia.
Se että olio on oman komponenttinsa kuuntelija, voi joskus mennä sottaiseksi (minulla on ollut ongelmia omassa projektissani sen kanssa), mutta en välttämättä uskoisi niin tapahtuvan tässä, koska Timerin tahdissa tapahtuvaa päivittämistä ei tehdä itse Refereessä ja päivittäminen ei näin ollen häiritsisi Timerin kuuntelua sen enempää vaikka kuuntelija sisältyisikin Referee-luokkaan.
StartTimerListener.java
StartTimerListener on ehkä hieman harhaanjohtavasti nimetty, koska sehän kuuntelee JButtonia aloitusvalikossa, ei varsinaisesti mitään Timer-oliota? Voi olla että ymmärsin jotain väärin, mutta käsitin niin että luokassa
Logic on startGame-metodi, jota StartTimerListener kutsuu jollain parametrilla, jolla alustetaan Logic-olion timeInSeconds-oliomuuttuja, jota vähennetään aina jokaisella update-metodin kutsulla yhdellä. Ja nämä kutsut tapahtuvat Refereen Timerin mukaan. Etsin koodista melko pitkään kahta Timer-oliota, vaikka loppujen lopuksi niitä näkyisi olevan vain yksi. Parempi nimi ehkä StartTimerListener-luokalle olisi NewGameButtonListener tjsp.
Testit
Ihan pieni sivuhuomio, mutta testeissä (esim. standingsListTest-testiluokka ja testit searchStandingsWithNameReturnCorrectStandings ja searchStandingWithNameReturnsNullWhenNotFound, oli käytetty erillistä boolean-muuttujaa sen tarkastamiseen, onko jokin muuttuja null. Tämä hoituu lyhyemmin kirjoittamalla assertNotEquals(muuttuja, null).