samhareem
commented
8 years ago Ladattu 09.10.2015, klo 14:03
- Ensi vilkaisulla /Bingo kansion sisällä olevat tekstitiedostot ovat erittäin sekavia. Saisiko näitä siirettyä ryhmiteltyä eri kansioihin (Esim. /bingot, /kayttajat etc.), ja nämä kansiot siirrettyä resurssikansioon.
- Voisiko myös ohjelman luokkia paketoida hieman enemmän. Nyt ainoastaan käyttöliittymäluokat on paketoitu, voisiko saman tehdä esim. logiikaluokille ja utility luokille.
- Metodien, muuttujien ja attribuuttien nimeämistä voisi vielä selkeyttää. Esim TiedostoKasittelija luokan metodit luoF voisi nimetä uudelleen luoTiedosto, onFOlemassa voisi nimetä tiedostoOnOlemassa. Samoin attribuutteja tiedsis ja lkm voisi avata hieman, esim tiedostonSisalto ja labelLukumaara. Myös luokka TiedYp on nimetty hyvin epäselvästi.
- Koodia voisi kommentoida enemmän. Itsekkin sorrun lähes aina alikommentointiin, mutta esim. luokassa bingopohja minulle jäi aluksi epäselväksi mitä attribuutti lkmmax tekee. Tähän liittyy myös edellinen huomio nimeämisestä.
- Metodit ovat pysyneet hyvän kokonaisina.
- Käyttis ja toiminnallisuus hyvässä kuosissa. En kohdannut bugeja.
ZIP ladattu 20.9. 12.59
Mikäli ymmärsin idean oikein, on se hyvin hauska: Readmen pohjalta tulkitsen, että kyseessä on tavallisen tyylinen bingo, jossa voisi luoda helposti omia bingoja - tyyliin eri tapahtumille omiansa -, joita voisi sitten jakaa ystäviensä kanssa ja näin jättää Euroviisuja ja muita tapahtumia varten väännettävät paperimallit historiaan. Pidän myös ajatuksesta tehdä mobiili-versio. Parhaimmillaan erinomainen lisä näin älypuhelinten aikakaudella ja bingon luonteen pelinä huomioiden!
Koodi on omaan silmääni pääosin nättiä: Säntillisesti sisennettyä ja mukavasti rivinvaihtoja tuomassa ilmaa eri koodipätkien välillä. Metodit nimetty suurimmilta osin kuvaavasti. Ruudukon konstruktorin voisi minusta jakaa useampaan metodiin, tyyliin nappien lisääminen paneeliin omaan metodiinsa ja kuuntelijoiden lisääminen omaansa. Samoin, mikäli nappien etiketit aiotaan tallentaa taulukkoon, voisi saman luokan actionPerformed-metodin ehkä typistää for-loopin avulla? Tai voisit ehkä koittaa jotenkin sutvia tarvittavat tiedot HashMapiin, esimerkiksi siten, että etiketti on avaimena ja nappi arvona, jolloin ei ehkä tarvittaisi toistoa ollenkaan tässä yhteydessä? En ole varma, onnistuuko kumpikaan fiksusti, mutta koodia lukiessa tulivat mieleeni. Suurimmilta osin minusta joka tapauksessa hyvää jälkeä!
Runsas kommenttien määrä on yleensä "koodihaju", mutta tässä niitä oli käytetty enimmäkseen järkevästi. Ohjelma kun on vielä keskeneräinen, niin lukeminen on välillä hankalaa. Esimerkiksi TiedostoKasittelija-luokan rooli olisi jäänyt minulle täysin mysteeriksi ilman muutamia kommentteja. Olivat lyhyitä, ytimekkäitä ja helpottivat tässä tapauksessa allekirjoittaneen hommaa. Valmiissa ohjelmassa tuskin ovat, eikä pidäkään olla enää samalla lailla tarpeen, jos selkeä koodi jatkuu. Koko lailla valmiissa metodeissa - esim. Bingopohjan muutaMerkki-, taulukkoEiTaysi- ja taulukkoVoittaja-metodit taitavat olla suunnilleen sitä, mitä tulevat olemaankin? - voisi kommentit minusta jo jättää pois.
Käyttöliittymä näyttää olevan jo muotoutumassa pikku hiljaa. Ulkomuoto toi mieleeni ristinollan, kun koko 3_3, vaikka readmessa puhutaan 5_5:stä? Pit-testeistä tosi päättelin, että ristinolla ilmeisesti oli alkuperäinen aihe? Käytännössähän ulkoiselta olemukseltaan ovat samanlaisia, joten toiminee hyvin tarkoitukseensa. Pientä asettelua, niin itse pelinäkymä on melkein valmis.
Testejä on aika vähän, mutta toisaalta niin on toistaiseksi testattavaakin. Vaikka lienee sanomattakin selvää, niin tähän kannattanee kiinnittää huomiota. Kaikki tehdyt testit menivät läpi. Pit-raportti oli hiukan vanhentunut, joten en oikein osaa mutaatioista tai rivikattavuudesta sanoa mitään tarkkaa.
All in all: Varsinaisesti isoja ongelmia en näe. Ehkä kevyttä laahaamista testien osalta, mutta korjaantunee, kun koodi lisääntyy. Uskon vakaasti, että hyvä tästä tulee ;)