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
virtalas opened this issue Apr 20, 2018 · 0 comments
Open

Koodikatselmointi #1

virtalas opened this issue Apr 20, 2018 · 0 comments

Comments

@virtalas
Copy link

Projekti ladattu 20.4. klo 16.48

Yleistä

  • Polun löytäminen on visualisoitu hauskasti :D
  • Huomasin että sieltä täältä puuttuu välilyöntejä esimerkiksi sulkujen ja aaltosulkujen väleistä. NetBeansissä saa helposti formatoitua Source > Format.
  • Ominaisuusehdotus: käyttäjä voisi valita kartan?
  • Luokkia on vielä vähän mutta kun niitä tulee lisää voisi harkita niiden organisointia eri paketteihin/kansioihin.
  • Tarvitseeko karttakansion (Maps) olla lähdekoodien kanssa samassa kansiossa? Voisi olla siistimpää jos se olisi jossain muualla sivummassa.
  • Projektissa on aika paljon massivisia if-else haaroja ja silmukoita. Saattaa olla että algoritmit ovat sellaisia että vaativat niitä, mutta koodista tulisi nätimpää jos haarat ja loopit pitäisi vähärivisinä ja haaraisina.
  • Projektia ei voi ajaa gradlella (run puuttuu?) mutta NetBeans käänsi ja ajoi hienosti.

PathFinding

  • Voisiko main-metodia yrittää pilkkoa vähän? Esimerkiksi while-loopin voisi siirtää omaan metodiinsa joka palauttaa askelien lukumäärän.

AStar

  • find() on aika pitkä. Saisiko siitä esim. loopit omiin metodeihinsa? Kun metodit vielä nimeää selkeästi niin find():n rakenne selkenee myös.

JPS

  • find() tässäkin aika pitkä.
  • Muuten metodit on selkeitä ja hyvin nimettyjä.
  • Onko sisäkkäisille if-lauseille muuta syytä kuin ehtojen selkeys? Vähän vaan hämäävän näköinen kun ehdot voisivat olla yhden if:n ehdossa, mutta ei mikään suuri juttu.
  • findNeighbors():nkin pilkkomista voisi harkita. Onhan se ihan selkeän näköinen, pitkä vaan.

Testit

  • AStarTest.testReconstructPath() pitäisi varmaan olla relatiivinen path niin kuin muissa. Nyt map on tyhjä joten testi ei taida mennä ihan oikein. Mutta miksiköhän se menee silti läpi...
  • Testeissä on vaan yksi yksikkötesti per metodi, mutta voisi testata kattavamminkin eri syötteillä ja skenaarioilla, ja testata kaikki ääripäät.
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