Mahtis / loganalyser

Application for analysing default Presentation logfiles into more manageable form
0 stars 0 forks source link

Koodikatselmointi #1

Open laitilari opened 7 years ago

laitilari commented 7 years ago

Latasin projektin 04.02.2017, kello 15.20.

Koska koodikatselmoinnissa tulee keskittyä vain itse koodiin, en puutu sen kummemmin aiheeseen tai muuhun, mutta sanon kuitenkin, että vaikuttaa todella käytännölliseltä ja hyvältä aiheelta!

Koodikatselmointi

Suoritus ja testit

Pystyin suorittamaan ohjelman ongelmitta. Esimerkkitiedostosta data siirtyi uuteen text2 tiedostoon onnistuneesti ja selkeämmän näköisenä. Ohjelman suorituksen jälkeen ajoin testit, mutta jostain syystä testit eivät menneet läpi. Kolme testiä epäonnistui:

https://gyazo.com/7f7b7ad8905aeb623d10534c37b261f2

En pureudu syvemmin siihen, mikä voisi olla syynä. Ehkä ne toimivat sinulla, mutta jostain syystä minulla eivät.

Koodi

ExperimentInfo.java, rivi 19 ja 120: voisit katkaista nuo rivit ja jatkaa seuraavalla rivillä. Menevät nyt turhan pitkälle, jos koodia lukee esim pienellä näytöllä tai pienessä ikkunassa, joutuu kelaamaan. Katso sama juttu myös muista luokistasi, ainakin Trial luokassa on myös samaa.

ExperimentInfoIO.java: useat metodit: Sovelluslogiikan metodien tulisi olla korkeintaan kymmenen riviä, mutta tässä luokassa on useita kymmeniä rivejä pitkiä metodeja. Luulen, että noita pystyisi kyllä jakamaan pienempiin osiin, esimerkiksi suorittamalla if:n jälkeinen koodi toisessa metodissa.

Onko copy paste koodia metodissa saveToFile, jossa luot usean StringBuilderin? Pystyisikö tämän jotenkin tehdä yhdellä SB:llä?

Luokka herättää kysymyksen single responsibility periaatteesta. Eli luokan tulisi hoitaa vain yhtä asiaa. Pitäisikö siis tiedostosta lataaminen ja tiedostoon tallettaminen tapahtua eri luokissa? Minusta kyllä tämä ratkaisu toimii hyvin näinkin, mutta tuo kysymys tuli mieleeni, mikä kannattaa ehkä selvittää ohjaajan kanassa.

LogParser.java: metodi parseIntoTrials: Sama juttu, kuin edellisessä kappaleessa. Jos vain mahdollista, niin voisi yrittää lyhentää tätä metodia.

Yleistä koodista

Olet nimennyt metodit selkeästi ja johdonmukaisesti. Olet kommentoinut koodin väliin, mitä metodit tekevät, mikä selkeyttää koodin lukemista. En löytänyt copy-paste koodia, paitsi ehkä aiemmin mainitusta kohdasta. Signle responsibility periaate näyttäisi olevan kunnossa. Luokat ovat selkeissä oman toimintansa pakkauksissa. Testien nimentä on selkeää ja testeja olet näköjään ehtinyt tehdä jo paljon.

Kaiken kaikkiaan oikein loogisen oloinen paketti. Suurimpana miinuksena eräiden metodien pituus, mutta kenties asia onkin jo työn alla.