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
Jhoneagle opened this issue Aug 29, 2018 · 0 comments
Open

Koodikatselmointi #1

Jhoneagle opened this issue Aug 29, 2018 · 0 comments

Comments

@Jhoneagle
Copy link

Jhoneagle commented Aug 29, 2018

Ladattu projekti 28.8.2018 klo. 19.14

Suurin ongelma, että projektia ei pystynyt testaamaan käytännössä, ajamaan testejä tai kokeilla jarin luontia. Tämä johtuu siitä, että projetilla ei ole releasia ja projektilla ei näytä olevan minkäänlaista ohjetta testikattavuuden, testien ja jarin suorittamiseen. Itse nimittäin en tunne gradle projekti tyyppiä ja ainakaan IDEA kehitysympäristöt eivät suostu buildaamaan jonkin gradle konfiguraation puutteen takia, kun omatoimisesti yritin.

Yleisesti ottaen testit näyttävän hyvin kattavilta ja projekti koodin puolesta näyttää huolella tehdyltä. Visuaalinen käyttöliittymä on ehdottomasti plussaa. Toivon mukaan käytännöllinen. Reitinhaku algoritmienkin logiikka on hiottu hyvin poistaen valta osan yleisistä ongelmista. Tosin tietorakenteissa ja olioissa olisi hyvä käsitellä kaikki mahdolliset virhetilanteet. Sen sijaan, että heittäisi vain erroreita. Esimerkiksi jos yrittäisi lisätä, mutta ei pysty, niin silloin voisi olla laittamatta sitä ja ilmoittaa lisäyksen epäonnistuneen. Tai jos yrittää etsiä väärän tyyppisellä oliolla, niin ilmoittaa, että haettavaa ei voi löytyä.

Muuten huomioita lähinnä kosmeettisessa mielessä ja selkeyden kannalta. PriorityNode ja Pair luokat näyttäisivät olevan lähinnä varsinaisen Node luokan toistoa pienellä erotuksella. Jälkimmäistä ei sinänsä edes käytetä, kuin pääasiallisesti Noden luontiin. Joten selkeyden kannalta PriorityNoden ja Pairin voisi hyvin sulauttaa Nodeen ja korvata niiden käyttö sillä.

Heuristiikan taas voisi hyvin siirtää abstractiin pathfinder luokaan. Koska niihin pääsee kaikista sen aliluokista eli varsinaisista algoritmien implementaatioista silloinkin, mutta ilman staattisuutta. Eikä tarvitsisi kutsua niitä erillisestä luokasta. Kun taas NeighbourRuleFactorin staattisen metodin voisi hyvin sisällyttää NeighboursRule rajapintaan. Sillä Java 8:sta lähtien rajapinnat voivat sisältää staattisia metodeja ja ne toimivat/näkyvät aivan samallalailla, kuin normaalissa luokassa olevat. Staattisia metodeja ei myöskään tarvitse implementoida rajapintaa käyttävissä luokassa vaan sen voi tehdä suoraan rajapinnassa. Mikä tärkeintä silloinkin se näkyy samanlailla, kuin muut metodit ulospäin.

Pienempiäkin yhdentämiskohtiakin on, mutta ne eivät varsinaisesti tuo eroa mitenkään. Esimerkiksi heapifyDown ja -Up. voisivat olla yksi sama heapify, mutta yhtä hyvin toimivat erilläänkin. Toivottavasti projekti ehtii mahdollisimman pitkälle, sillä kurssi suorituksena kokonaisuus näyttää erittäin hyvältä.

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