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
algoholik opened this issue May 4, 2021 · 0 comments
Open

Koodikatselmointi #1

algoholik opened this issue May 4, 2021 · 0 comments

Comments

@algoholik
Copy link

Lähdekoodi ladattu: ti 4.5.2021 klo 13:40

Palaute:

  1. Ohjelmalogiikka, käyttöliittymä, tietokannan käsittely sekä luokka-abstraktiot on asianmukaisesti eriytetty toisistaan omiksi tiedostoikseen.

  2. Funktiot, metodit, muuttujat ja parametrit on nimetty selkeästi ja kuvaavasti snake_case -formaattia käyttäen.

  3. Luokat on nimetty osuvasti PascalCasella.

  4. Funktioiden ja metodien vastuualueet ovat asianmukaisen "atomisia", eli niillä on selkeä yksi vastuualue. Ne on myös nimetty osuvasti vastuualuetta kuvaaviksi.

  5. Projektikansiossa ei ollut turhia suorituksenaikaisia tiedostoja tai kansioita, eli sen .gitignore oli kunnossa.

  6. Sovellukselle ei ollut lainkaan testejä, vaikka komennot "poetry run invoke test" ja "poetry run invoke coverage-report" pystyi suorittamaan onnistuneesti. Testikattavuus oli toisin sanoen 0%.

Parannusehdotuksia:

  1. Sovelluksen koodikannan kasvaessa sen logiikan eri vastuualueita voisi alkaa jakamaan omiin kansioihinsa ja mahdollisesti useampaan eri tiedostoon.

  2. Moduulien importtaamisessa (tuomisessa?) voisi vähentää tautologiaa kirjoittamalla moduulin nimen vain kerran, jonka jälkeen määriteltäisiin yksittäiset tuotavat luokat pilkuilla eroteltuna ja tarvittaessa sulkeissa, esim: "from moduuli import (luokka1, luokka2, luokka3)".

  3. Funktioissa, luokissa ja niiden metodeissa voisi ottaa käyttöön PEP 484:ssä määritelty "type hints", kts. https://www.python.org/dev/peps/pep-0484/ sekä näppärä cheat sheet: https://mypy.readthedocs.io/en/stable/cheat_sheet_py3.html

  4. Koodissa käsitellään paljon merkkijonoja (strings) ketjutusta (concatenation) käyttäen. Tässä olisi teoreettinen mahdollisuus optimoida koodin tehokkuutta siirtyä käyttämään PEP 498:ssa esiteltyä merkkijonojen käsittelytapaa "Literal String Interpolation" (tuttavallisemmin f-stringit), kts. https://www.python.org/dev/peps/pep-0498/ - vaikkakin myönnettäköön että sovelluksessa käsitellyt datamäärät ovat mittaluokaltaan niin pieniä ettei tällaisella optimoinnilla juuri olisi muun kuin koodin luettavuuden kannalta olennaista merkitystä.

  5. Koodin - etenkin funktioiden ja metodien - yksityiskohtaisempaan kommentointiin voisi panostaa enemmän, vaikka ne ovatkin sinällään varsin hyvin nimetty.

  6. Ymmärrettävästi sovellus on vielä kehitysvaiheessa ja siksi katselmointihetkellä oli käytössä vain tekstikäyttöliittymä. Ohjelman käytettävyys tulee nousemaan huomattavasti jos ja kun sovellus saa vaatimusmäärittelyssäkin luonnostellun graafisen käyttöliittymän.

Loppusanat:

Yleisesti ottaen koodi oli selkeää luettavaa. Sovelluksen tekstikäyttöliittymä toimi kuten pitikin, ja oli toteutettu mielestäni niin hyvin kuin moisen sovelluksen tekstikäyttöliittymän nyt voi ylipäätään toteuttaa. Graafista käyttöliittymää odotellessa, tsemppiä jatkokehitykseen! Muista alkaa testaamaan koodia ajoissa!

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