cakerr / sudoku Goto Github PK

Hei!

Käytin tämän koodikatselmoinnin tekemiseen sovelluksesi versiota, jonka latasin 3.4.2015 klo 21:35.

Tässä ranskalaisilla viivoilla tekemiäni huomioita:

• Ohjelmasi tuntui olevan vielä aika alkuvaiheessa. Kannattaa tehdä ohjelmaa melko säännöllisesti, jotta sinulle ei lopussa iske kova kiire ja stressi.
• Koodisi oli hyvin sisennettyä, mikä teki lukemisesta helppoa. Jatka samaan tapaan!
• Olet myös nimennyt metodit loogisesti niin, että jo ennen metodin lukemista on melko helppoa ymmärtää, mitä metodi tekee. Hyvä! Olethan myös huomannut, että sinulla on muutama metodi tällä hetkellä toteuttamatta, vaikka ne on nimetty (duplikaattimetodit)?
• Pidän myös tavalla, jolla olet luonut ensimmäiset luokat. Ne vaikuttavat selkeän tehtävän omaavilta.
• Tällä hetkellä sudokuruudukkosi on 9x9-kokoinen int-taulukko. Oletko harkinnut, että kentän voisi mahdollisesti myös koostaa yhdeksästä 3x3-ruudukosta? Nuo 3x3-ruudukot ovat tärkeitä yksiköitä sudokussa, ja nyt tuon suuren ruudukon pyörittely vaikuttaa hieman vaivalloiselta. Jos nämä pienemmät ruudukot tikkaisi hyvin yhteen, voisi mm. tarkistaminen helpottua. Kuitenkin, jos itse koet, että suuri taulukko toimii sinun ajatuksissasi paremmin, niin go for it!
• TarkistajaTest -luokasta voisit poistaa käyttämättömät metodit.
• Samaisessa testiluokassa kaipaisin myös metodien kaikkiNumerotRuudukossaPalauttaaFalse ja KaikkiNumerotVaakarivillaPalauttaafalse nimiin selvennyksen siitä, miksi niiden odotetaan palauttavan false (vrt. metodi oikeatNumerotPalauttaaFalseKunNegatiivisiaArvoja). Jokaisessa testissä, jossa odotat jonkin asian menevän pieleen, olet kuitenkin etukäteen säätänyt tilanteen, jossa asiat menevät tietyllä tavalla mönkään.
• Tykkään tavasta, jolla olet käyttänyt Tarkistaja-luokassa metodeja. Et ole luonut liian isoja metodeja, vaan hyvän tavan mukaisesti jakanut asioita pienempiin palasiin.
• Voisit jo ohjelmoidessa luodan Main-luokan, jonka kautta testaat ohjelman toimintaa käyttämällä sitä, vaikka sinulla ei vielä käyttöliittymää olisikaan. Asioiden kokeileminen ja ruudulle printtaaminen auttavat varmistumaan siitä, että olet oikeilla raiteilla.
• sarakkeetOikein-metodissa if-ehto ei ole oikein. Nyt metodi palauttaa false, silloin kun sarake sisältää kaikki numerot, eli oikeellisessa tilanteessa.
• Käytät Tarkistaja-luokassasi monia hienoja oivalluksia (kuten kolmella jaollisuutta).
• Sinun kannattaa JavaDocissassi (kun sellaisen luot) kiinnittää erityistä huomiota Tarkistaja-luokan metodeihin. Vaikka olit nimennyt ja ryhmitellyt metodit hyvin, kesti minulta hetken päästä kärryille luokan toiminnasta rivitasolla. Kannattaa helpottaa lukijan työtä.

Olet hyvällä alulla, nyt vain rohkeasti eteenpäin!

Ladattu 29.4.2015 14:22

• Kommentointia oli hieman sekavasti, ilmeisesti javadocin kirjoittaminen on vielä kesken. Jotkin metodit olisi kaivanneet täsmällisempää kommentointia, sillä nyt oli hieman hankalaa hahmottaa, mitä metodi oikeastaan tekee tai mihin se liittyy. Myös englannin ja suomen sekoittaminen hieman häiritsi.
• Luokat oli kuitenkin jaettu selkeästi ja loogisesti. Vaikutti siltä, että niillä kaikilla oli oma vastuunsa, mitä ne noudattivat hyvin. Jotkin metodit olivat ehkä hieman "isoja", mutta pääasiassa koodi kuitenkin noudatti hyvin clean code periaatetta.
• Koodissa oli mielestäni onnistuttu erottamaan käyttöliittymä ja pelilogiikka hyvin toisistaan, mikä ainakin minulle itselleni on ollut hyvin vaikeata. Lisäksi myös listenereille tehty oma luokka selkeytti koodin lukemista paljon, koska niitä oli melko monta. Itse en ollut tullut ajatelleeksi, että ne voisi tietenkin laittaa omaan luokkaan.
• Testit vaikuttivat muiden luokkien osalta oikein kattavilta, mutta hieman ihmetytti, ettei sudokuluokalle ollut kirjoitettu testejä. En tiedä onko luokka tarkoitus jättää testaamatta vai eikä niitä vain ollut ehditty vielä kirjoittaa?
• Peli käynnistyi ongelmitta ja näytti ihan hyvältä. Koko sudokua en viitsinyt ruveta alkaa ratkaisemaan. Ilmeisesti jotain toiminnallisuutta (uuden kentän tekeminen ja ratkaise-toimnto) eivät vielä ole käytössä, mutta kaikenkaikkiaan peli näytti olevan oikein hyvällä mallilla :)

Hei,

ensimmäiseksi pakko sanoa että hienon näköistä koodia, todella hyvin olet noudattanut clean code periaatetta. Ainoa mikä ehkä pisti silmään oli Tarkistaja-luokka, siinä itse olisin yhdistänyt osan metodeista, mutta näin ainakin testaus on varmasti helppoa.

Toinen pieni huomio koodin ulkonäöstä; englantia on sekoittunut jonkin verran muuten suomenkieliseen koodiin, mikä näyttää aina hieman kyseenalaiselta (mm. swapKentat).

Testaus näytti hyvältä mutta sitä ei tainnut vielä olla kaikille luokille, tosin kaikki toiminnallisuudet eivät tainneet vielä olla täysin valmiitakaan, kuten uuden sudokun generointi, joka ainakin tässä päässä heittää kaikkiin ruutuihin nollan (tyhjä?).

Toinen bugi jonka huomasin oli ratkaise toiminnossa ratkaisu ei muuta jo täytettyjä kenttiä oikeiksi, eli jos kaiken täytti vitosilla niin se oli "oikea " vastaus.

Graaffinen käyttöliittymä näytti hyvältä ja sitä oli helppo käyttää intuitiivisesti. Ainoa parannusehdotus minkä keksin on että valittu kenttä näkyisi jollain tavalla valittuna kun sitä on klikannut.

Kaiken kaikkiaan ohjelma on mielestäni hyvällä mallilla. Vielä kun viimeisetkin toiminnallisuudet on lisätty niin se on erittäin tyylikkäästi toteutettu sudoku-peli!