Koodikatselmointi

Projekti ladattu 16.6.2020 klo 18:23

Aloitin koodikatselmoinnin tutustumalla ensin sovelluksen dokumentaatioon, minkä jälkeen siirryin testaamaan sovellusta Herokussa. Näiden jälkeen siirryin tutkimaan sovelluksen koodia tarkemmin. Dokumentaatio näyttäisi olevan ainakin lähes valmis. Katsoin myös tietokantakaavion läpi, ja huomasin, että Project-taulu kaaviossa ei vastaa täysin toteutusta: toteutuksessa ei ole saraketta budget vaan sarake active. Tätä ei toisaalta ehkä vain ole vielä päivitetty loppupalautusta varten.

Yleiskuva Herokusta

Sovellusta on helppo käyttää, eikä sovellusta käyttäessä tule tunnetta, että ei tiedä, miten jokin asia tehdään. Sovellus on myös muistettava ja sovellus toimii lähes kaikissa testaamissani tilanteissa oletetulla tavalla. Sovelluksen ulkoasu on siisti, mutta räikeän punainen väri on aika raskas silmille. Herokua varten toivoisin dokumentaatioon muutamat testitunnukset, jotta sovelluksen testaaminen helpottuisi.

Uutta skilliä lisätessäni huomasin, että kun syöttää virheellisen arvon kenttään, jossa kysytään kokemusta vuosina, tulee virheilmoitus, jossa vuosien pyydetään olevan 0-50 kuukautta (experience years should be between 0-50 months). Tässä pitäisi ehkä lukea, että 0-50 vuotta? Kuukausien tulisi puolestaan olla 0-1000, mutta jäin miettimään, riittäisikö rajata hyväksytty kuukausien määrä 0-12 tai 0-11, sillä 12 kuukauden jälkeenhän kokemusta on jo kertynyt vuosi.

Valikon Find people-linkin takana oleva listaus näyttää siltä, että se voi kasvaa todella pitkäksi, jolloin selaaminen käy käyttäjälle raskaaksi. Tässä voisi ehkä auttaa sivutus, jolloin yhtä aikaa ei näytettäisi niin paljon tietoa kerralla. Hakutoiminnallisuus toisaalta auttaa jo paljon, jos tietää tarkalleen, mitä etsii. Samoin skills-listaus yksittäisen käyttäjän kohdalla voi ilmeisesti kasvaa todella suureksi, joka sekin lisää selattavan tiedon määrää. Riittäisikö esimerkiksi se, että Find people-sivulla näkyisi vain lyhyesti kunkin käyttäjän kohdalla osaaminen listana (esim. "React, Java, Python, JavaScript") ja tarkemmin tietoja voisi tarkastella henkilön profiilista?

Kävin läpi kaikki README.md:ssä luetellut päätoiminnallisuudet, ja ainoastaan yhdessä niistä huomasin puutteita: kun kokeilin päivittää erästä skilliä niin, että laitoin kokemuksessa vuosien määräksi negatiivisen luvun, sivu vaihtui tyhjäksi, eikä minkäänlaista virheviestiä tullut näkyviin. Käytännössä sivulta katosi kakki omat skillsit. Muuten päätoiminnallisuudet toimivat hyvin!

Koodi

Sovelluksen rakenne on selkeä ja helppo ymmärtää. Koodi on myös selkeää, ja muuttujien nimentä on johdonmukaista ja kuvaavaa. En huomannut poiskommentoitua koodia ja sisennykset näyttävät myös olevan kunnossa.

Sovelluksessa ei vaikuta olevan kuin yksi käyttäjäryhmä. Toisaalta sovellus ei välttämättä sellaista edes tarvitse, mutta en ole aivan varma, onko arvosteluperusteissa mainintaa erilaisista käyttäjäryhmistä. Jos esimerkiksi ylläpitäjä-käyttäjän haluaa vielä toteuttaa, kurssimateriaalissa on mielestäni hyvät ohjeet sen toteuttamiselle!

Huomasin koodia tarkastellessani, että sovelluksessa on muutamia tyhjiä tiedostoja. Tyhjät tiedostot ovat accounts/_ init _.py, auth/_ init _.py, projects/_ init _.py ja skills/_ init _.py. Jos nämä ovat turhia, ne kannattanee poistaa ennen loppupalautusta.

Kun yrittää päästä osoitteeseen https://skill-em-all.herokuapp.com/accounts/2 niin, että kirjautuneen käyttäjän id ei ole 2, heittää sovellus käyttäjän kirjautumissivulle. Kirjautumissivun näyttäminen tuntuu tässä tilanteessa hieman hassulta, kun on jo kirjautunut sisään. Olisiko parempi palauttaa alla olevassa koodin kohdassa (application/accounts/views.py:20) jonkinlainen access denied -virhesivu?

if not __is_owner(account_id):
      return login_manager.unauthorized()

Sovelluksessa on mahdollista päästä tarkastelemaan muiden käyttäjien projekteja kirjoittamalla haluttu osoite osoitekenttään (esim. https://skill-em-all.herokuapp.com/projects/2), vaikka käyttäjätarinoissa ei tällaisesta ominaisuudesta puhuta ja sovelluksessa ei näyttäisi olevan muille käyttäjille mahdollisuutta tarkastella muiden projekteja. Tämä saattaa olla tarkoituksellista, mutta jos näin ei ole, lisäisin myös metodiin projects_manage(project_id) tarkistuksen siitä, että onhan kirjautunut käyttäjä todella luonut projektin, jota hän yrittää tarkastella.

Herokussa sovellusta testatessani ihmettelin, miksi vuodet pitäisi ilmoittaa väliltä 0-50 kuukautta. Koodin perusteella toisaalta on ehkä aika turvallista sanoa, että tässä on vain käynyt validoinneissa pieni huolimattomuusvirhe ja todellisuudessa tarkoitetaan 0-50 vuotta.

Hyödynsin koodia tarkastellessani myös arvosanan 5 minimivaatimuksia. En huomannut koodissa vielä toista yhteenvetokyselyä, mutta sen ehtii vielä toteuttaa. Saattaa myös olla, että en vain huomannut sitä. Kaikkiaan koodi on selkeää ja sitä oli mukava käydä läpi.

HTML-tiedostot

Sisennykset näyttäisivät olevan kunnossa myös täällä. Koodi on selkeää eikä poiskommentoitua koodia ole. Kävin tiedostot läpi ACheckerillä, ja se huomautti joistain virheistä HTML-koodissa useissa eri tiedostoissa, joten en lähde niitä nyt tähän listaamaan, mutta suosittelen kuitenkin tarkistamaan seuraavat tiedostot ACheckerillä: macros.html, layout.html, skills_form.html, skills.html, projects.html, project.html, login_form.html, accounts.html, account_profile.html ja account_form.html.

Kokonaisuus

Sovellus on selkeä käyttää ja myös koodi on selkeää ja helposti seurattavissa. Sovellus toimii ja sitä on helppo käyttää. Aihevalinta on mielestäni onnistunut ja sovellus vaikuttaa hyödylliseltä. Hyvää työtä!

MiguelSombrero / skill-em-all