Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Koodikatselmointi #1

Open
janimaki opened this issue Apr 13, 2014 · 0 comments
Open

Koodikatselmointi #1

janimaki opened this issue Apr 13, 2014 · 0 comments

Comments

@janimaki
Copy link

Ladattu 13.4.2014 15:25

Alkuun huomio kiinnittyi luokkien ja pakettien nimiin, jotka olivat selkeitä ja informatiivisia, mutta minua alkuun vähän hämäsi luokat valmisPokerikasi ja valmiitPokerikadet. En tiedä sitten olisiko "esimerkkikädet" tai "voittavatkädet" yhtään parempia. Testit menivät läpi ja pit-raportin perusteella kattavuus oli todella hyvä. Ohjelman rakenteesta pääsi helposti perille.

Huomioita Kortti- ja Korttipakka-luokista:

Kortin maan voisi tehdä myös enumin avulla. Eipä siinä muuta huomautettavaa ole, näyttää toimivalta ratkaisulta. Metodien nimet näytti varsin ymmärrettäviltä ja selkeiltä.

Huomioita Pelaaja-, Pelivaraukset- ja Pokerikasi-luokista:

Pelaaja-luokassa metodien nimissä pokeritermistöä, mikä voi aiheuttaa vaikeuksia koodin luvussa joillekkin (getHanska, getRollit). Luokassa Pokerikasi näyttäisi olevan ylimääräinen (turha) konstruktori ja metodi vaihdaKortti() turhaan poistelee pakasta kortteja, koska korttipakka- luokassa kortti on jo poistettu pakasta, kun se on nostettu sieltä. Pelivaraukset-luokan poistaPanos-metodi on vähän hassun niminen. Voisi olla vahennaPanosPelivarauksista tjsp.

Huomioita ValmisPokerikasi-, KadenTunnistaja ja ValmiitPokerikadet-luokista:

ValmisPokerikasi-luokasta voisi poistaa arvojärjestys-muuttujan, koska se on kuitenkin sama kuin kerroin, joten kerrointa voisi käyttää sen tilalla. Aloin myös miettimään tarvitseeko käsiä edes vertailla compareTo-metodin avulla?
KadenTunnistaja-luokka on vielä kesken, mutta oletettavasti sinne tulee metodit jokaiselle eri kädelle. Sinne voisi tehdä metodin onkoVoittoa(), joka palauttaisi true/false, niin ei erikseen tarvitsisi tarkastella haita ja paria. Nythän metodi onkoHai() palauttaa true myös suoralle ja värille ainakin tällaisenaan.
KadenTunnistaja voisi myös suoraan jokaisen käden metodissa asettaa kertoimen voitolle, jolloin ValmisPokerikasi- ja ValmiitPokerikadet-luokkia ei välttämättä edes tarvittaisi. Esim onkoVari-metodi palauttaessaan true asettaisi kertoimen värille.

Muuta:

Koska ohjelmaa ei voi suorittaa (tai mitään ei tapahdu), on bugien etsiminen melko vaikeaa, olisi kiva nähdä peli jonkinlaisen käyttöliittymän kanssa, joten mielenkiinnolla sitä odotellessa.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant