Vertaisarviointi / Aineopintojen harjoitustyö: tietorakenteet ja algoritmit
Closed this issue · 2 comments
Vertaisarviointi
Repositorio ladattu 7.10.2020 klo 18:15
Yleisiä huomioita
En saanut ohjelmaa käynnistettyä sen enempää komentoriviltä kuin NetBeansistakaan. Ongelma liittyy jotenkin JavaFX:ään, enkä saanut sitä ratkaistua. Olisikohan JavaFX:n hyvä olla määritelty projektin riippuvuuksiin?
Yritin keksiä joitain parannusehdotuksia koodista, mutta joka tapauksessa projekti vaikutti oikein hyvältä. Myös käyttöliittymään on selvästikin nähty vaivaa. Harmi etten päässyt testaamaan sitä!
Koodin selkeys ja luettavuus
Koodi on jaettu selkeisiin paketteihin ja luokat nimetty osuvasti. Arvioijan on siis helppo saada yleiskuva sovelluksen rakenteesta.
Koodi on muutenkin mielestäni selkeää, ja metodit ja muuttujat on nimetty kuvaavasti. Seassa on joitain englanninkielisiä muuttujien nimiä (now, later).
Algoritmien päämetodeille voisi ehkä antaa jonkin kuvaavamman nimen kuin algo – esim. etsiReitti, haeReitti tai ehkä suorita? Nämä metodit ovat myös varsin pitkiä, mikä taitaa kyllä olla tällaisten algoritmien kohdalla aika tavallista eikä koodin luettavuus siitä juurikaan kärsi.
Joissain kohti koodista saisi mielestäni luettavampaa, jos arvoja tallettaisi välillä muuttujiin. Fringe-luokassa näin tehdäänkin rivillä int x = solmu.getX().
Sitä vastoin esim. rivi
cache1[ruudukko.getJarjestysnumero(ruudukko.getAloitus().getX(), ruudukko.getAloitus().getY())] = 0;
on melko raskaslukuinen. Mietin myös, voisiko useassa kohdassa käytetty metodi getJarjestysnumero ottaa parametrina x- ja y-koordinaattien sijasta suoraan Solmu-olion? Tai sitten metodia voisi kuormittaa niin, että sitä voisi kutsua sekä Solmu-oliolla että koordinaateilla.
Ruudukko
Ruudukko-luokka sisältää paljon sovelluslogiikkaa. Osa logiikasta voisi olla hyvä siirtää erillisiin luokkiin, etenkin koska jotkut metodit eivät mielestäni liity suoranaisesti itse Ruudukkoon (esim. piirraReitti).
Kaiken kaikkiaan luokassa on kuitenkin oikein selkeitä ja mukavan lyhyitä metodeja.
Tyyppi
Tyyppi-enum on mielestäni kätevä ratkaisu. Luokkaa tarkastellessa ei kuitenkaan ole aivan ilmeistä, mitä tyypit FRINGE ja DIJKSTRA tarkoittavat eli mitä merkitsee, jos Solmu on tätä tyyppiä. Muut tyypit on nimetty oikein kuvaavasti.
Fringe
Tämä oli itselleni uusi ja mielenkiintoinen reitinhakualgoritmi.
Omasta mielestäni while-looppi olisi selkeämpi, jos loydetty-lipun sijasta if (ruudukko.getTyyppi(x,y) == Tyyppi.MAALI) -ehtolauseen sisällä aloitettaisiin reitin muodostamiseen liittyvät lopputoimet ja sitten poistuttaisiin metodista returnilla, mutta tämä on varmaan makuasia.
Testit
Testit vaikuttivat järkeviltä. Joissain testeissä menee AssertEqualissa väärinpäin odotettu ja saatu arvo, mutta niiden järjestys onkin aika epäintuitiivinen. Testien virheilmoitukset ovat toki selkeämpiä, jos nämä ovat oikeinpäin.
Jos kiinnostaa vielä käydä kokeilemaassa, niin aseta jdk 8 oletuskseksi :). Iso kiitos palautteesta, tästä on hyvä lähteä kehittämään.
Jep, kiitos vinkistä, sillähän se lähti käyntiin ja toimii oikein hyvin! 👍 Tuossa on tosiaan hieno, kun voi itse piirtää kartan, eikä vain valita valmiista vaihtoehdoista niin kuin monissa muissa projekteissa :)