matiastamsi
commented
4 years ago Tosi hyviä ehdotuksia! Suurin osa näistä on ollut tiedossa, mutta osaa en ole edes huomannut/tajunnut. Lisäksi ne mitkä olen huomannut hiukan ongelmallisiksi osoittautui palautteesi ansiosta ongelmallisemmiksi. Kiitos näin hyvästä palautteesta!
Ladattu 23.4. klo 12:00
Mielenkiintoinen peliaihio! Mielestäni ohjelmassa on hyvin jaettu ohjelman osia erilllisiin luokkiin ja perintää on käytetty hyvin helpottamaan pelin elementtien hallintaa.
Käyttöliittymän ja sovelluslogiikan eriyttäminen
Nähdäkseni sovelluslogiikka ja käyttöliittymä eivät ole nyt hirveän hyvin eriytettyinä. Sovelluslogiikka nojaa toiminnassaan käyttöliittymän komponenttiin pane, sekä toteuttaa toimintoja, jotka kuuluisivat käyttöliittymätoiminnallisuuksiin. Sovelluslogiikan elementit ovat suoraan käyttöliittymän elementtejä, mikä ei ole välttämättä viisasta. Koitan seuraavaksi parhaani mukaan avata:
Esimerkiksi Rapids-luokan metodi flow() tekee asioita käyttöliittymän puolesta, sillä se hoitaa kuvan uudelleenpiirtämisen AnimationTimer:lla. Siirtäisin AnimationTimer:n käyttöliittymään ja jättäisin ne asiat, jotka AnimationTimer jokaisella suorituksellaan tekee osaksi Rapids-luokkaa. Siis että käyttöliittymässä olisi toteutettuna AnimationTimer, joka kutsuu Rapids-luokan metodia flow(), joka ainoastaan laskee elementtejä eteenpäin yhden ruudun verran. Käyttöliittymä siis pyytää jokaisen ruudun erikseen Rapids-luokalta.
Toinen ongelma on, että nyt Rapids käyttää suoraan käyttöliittymän elementtiä pane. Tämä ei ole hyvä, sillä nyt sovelluslogiikka ei yksinkertaisesti voi toimia ilman käyttöliittymää, joka tarjoaa sille tämän pane-olion. Entä, jos sovellukselle haluttaisiin tehdä käyttöliittymä ilman JavaFX:ää? Ongelma on mahdollista kiertää siten, että myös muutokset pane:en tehdään käyttöliittymäluokassa FlyfishingUi, esimerkiksi yllä mainitun AnimationTimerin osana. Siis Rapids-luokkaan jäisivät listat kuplista ym. sekä metodi noiden listojen läpikäymiseen ja kuplien siirtämiseen askeleen eteenpäin. Nyt FlyfishingUi:n AnimationTimer kutsuu Rapids-luokan metodia flow(), saa vastauksena päivitetyt listat ja noiden listojen perusteella päivittää panen. Nyt Rapids:in ei tarvitse huolehtia käyttöliittymän päivittämisestä.
On vielä hieman ongelmallista, että sovelluslogiikan käyttämät elementit, kuten Bubble ja Leaf käyttävät javaFX:n käyttöliittymäkomponentteja. On tarpeetonta, että esimerkiksi Bubble:n BlackCircle ja WhiteCircle tietävät olevansa JavaFX:n Circle-olioita. Niille riittäisi tietää niiden keskipiste ja koko (ja väri?). Loppu piirtäminen voidaan jättää käyttöliittymän huoleksi, sillä tuohon piirtämiseen riittävät nuo tiedot. Näin taas helpottuu jonkin uuden käyttöliittymän rakentaminen saman sovelluslogiikan päälle. Eli kun käyttöliittymä saa Rapids-luokalta nuo bubbles ja leaves jne. -listat, niin se luo jokaiselle kuplalle sopivan javaFX-olion (tai oliot), eli esim kuplalle mustan ja valkoisen pallon, ja lisää ne pane:en. Käytännössä siis tehdään samat asiat, mutta käyttöliittymäkomponenttien käsittely on jätetty käyttöliittymälle.
Hyviä ohjenuoria tämän saavuttamiseen voisi olla ainakin se, että sovelluslogiikka ei saa käyttää yhtään javaFX-komponenttia, tai muitakaan käyttöliittymän komponentteja. Käyttöliittymä voi toki antaa tietoa sovelluslogiikalle, ja tämä on usein välttämätöntä (esim. mihin kohtaan käyttäjä klikkasi ruudulla.) Lisäksi käyttöliittymän ja sovelluslogiikan yhteistoiminnassa kannattaa noudattaa seuraavan kaltaista tapahtumaketjua:
-> (käyttöliittymä huomaa tapahtuman, esim klikkaus ) -> käyttöliittymä kerää ja välittää tarvittavat parametrit (esim. klikkauksen sijainti) logiikalle -> käyttöliittymä pyytää logiikalta toimintaa (esim. joen elementtien uusien paikkojen laskeminen, flow-metodi) -> logiikka suorittaa toiminnon -> logiikka palauttaa päivitetyt tiedot käyttöliittymälle (voi olla myös esim. jaettu olio/muuttuja palauttamisen sijaan) -> käyttöjärjestelmä piirtää päivitetyn kuvan käyttäjälle
Näitä tapahtumaketjuja voi olla käynnissä useita päällekkäin ja ne voivat olla eri vaiheissa, jos esimerkiksi ohjelma on toteutettu niin, että sovelluslogiikka ja käyttöliittymä ovat eri prosesseja ja sovelluslogiikka voi tehdä laskentaa taustalla.
FlyfishingUi ja StartPage
On hyvä, että käyttöliittymääkin on paloiteltu erillisiin luokkiin. En kuitenkaan tiedä, onko viisasta, että StartPage laitetaan näkyviin ennen, kuin startButton:in tapahtumakäsittelijä on lisätty. Tässä tapauksessa siitä ei seuraa katastrofia, mutta uskoisin, että on parempi lisätä käsittelijä ennen kuin StartPage kutsuu omaa show():taan. Näin käsittelijä varmasti tulee lisättyä ennen kuin käyttäjän on mahdollista painaa start-nappia.
Konfiguraatiot
Jos tekemisen puute iskee, niin olisi vielä suotavaa, että sovelluksen käyttämät konfiguraatiot, kuten ikkunan koko ja kuplien määrä eivät ole kovakoodattuja, eli että niiden arvot on suoraan kirjoitettu koodiin. Nämä avainparametrit voi määrittää nk. konfiguraatiotiedostossa, tähän löytyy ohje materiaalista. Kun parametrit on määritetty tällä tavoin erillään, niin niitä on helppo tarvittaessa muuttaa ja ei tarvitse huolehtia, että pitiköhän tämä vaihtaa johonkin muuallekin koodiin.
Testit
Testit vaikuttavat kohtalaisen kattavilta. Luokasta RockFactoryTest huomasin, että siinä testataan, että ovatko kaksi luotua kiveä erilaiset. Periaatteessa on mahdollista, että rng tuottaa samat luvut peräkkäin ja kivistä tulee samanlaiset, ja testi epäonnistuu. RockFactorylle voisi esimerkiksi antaa rng-olion parametrina, ja siten testatessa sille voisi antaa sellaisen rng-olion johon on asetettu tunnettu seed. Riippuvuuksien injektoinnista