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

1. koodikatselmointi #1

Open
Redande opened this issue Sep 24, 2015 · 0 comments
Open

1. koodikatselmointi #1

Redande opened this issue Sep 24, 2015 · 0 comments

Comments

@Redande
Copy link

Redande commented Sep 24, 2015

Tiedosto ladattu 24.9. klo 22:30

Valitsemasi aihe vaikuttaa hauskalta ja omaperäiseltä, ja ennen kaikkea hyödylliseltä tulevaisuuden käyttöä ajatellen.

Koodiin tekemäsi kommentit sekä selkeät luokkien, metodien ja muuttujien nimet tekevät koodista helppolukuista, jonka ansiosta oli helppo ymmärtää mihin pyrit missäkin koodinpätkässä.

Voisit ehkä kuitenkin jakaa koodia osiin paremman luettavuuden kannalta. Ehdottaisin ainakin funktion main_page siirtämistä omaan tiedostoon, johon voisi jatkossa toteuttaa muita käyttöliittymään liittyviä asioita.

Lisäksi rivin 67 if-lause on hyvin pitkä. Se kannattaisi ehkä rikkoa pienemmiksi palasiksi, jos mahdollista. Sen saisi helppolukuisemmaksi jos esimerkiksi määrittelisit ehdossa olevat minimit ja maksimit omiksi muuttujiksi ennen if-lausetta.

En käsittänyt täysin, miten funktio seek_point toimii, mutta haittaako, että rivin 67 if-lauseesta puuttuu vertailu siitä, että käyttäjän antaman pisteen latitude olisi suurempi kuin pointin 1 ja pointin 2 välisten latitudien minimi (eli haittaako, että puuttuu rimpsu: min($point1->lat, $point2->lat) < $user_point->lat).
Nyt siinä käsittääkseni tarkisetaan, että käyttäjän pisteen longitude on pointin 1 ja 2 longitudien välissä, mutta latitudessa tarkistetaan vain, että käyttäjän pisteen latitude on pienempi kuin pointin 1 ja 2 latitudien maksimi, mutta alarajaa ei oteta huomioon.

Tietorakenteiden ja algoritmien puolesta valitsemasi array map_pointeille vaikuttaa tarkoitukseen sopivimmalta rakenteelta, ja lisäksi toteuttamasi algoritmi pisteiden vertailuun seek_pointissa vaikuttaa nopealta.

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