Mahtis / loganalyser

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

Koodikatselmointi #2

Open laimikko1 opened 7 years ago

laimikko1 commented 7 years ago

Ohjelma ladattu 17.2 klo 20:14

Ohjelman rakenne on jaettu selkeästi eri paketteihin, domain, gui ja logiikka on eroteltu toisistaan. Samoin testipaketeissa jako tehty samalla tavoin hyvin.

Luokat on nimetty kuvaavasti ja JavaDoc toteutettu täsmällisesti. Näin ohjelman aihealueeseen vihkiytymättömänäkin saa hyvän kuvan siitä miten ohjelmasi rakentuu. Myös metodien nimeäminen on johdonmukaista ja toteutettu hyviä käytäntöjä noudattaen.

Luokkien toiminnallisuus on jaettu hyvin osa-alueisiin, jotka muodostavat yhdessä paketin kuvaaman toiminnallisuuden . Domain-luokassa olet erotellut hyvin yksittäisen henkilön vastaukset, kokeen/testin ja niiden muodostaman kokonaisuuden. (Jos ymmärrän termistöä oikein) Olet hyödyntänyt myös oivallisesti konstruktorin kuormittamista hyödyksesi ExperimentInfo-, ResponseMapping- ja Trial-luokissa.

Logiikka-luokassa log-tiedostojen kirjoittaminen ja parsiminen, sekä tiedoston lukeminen ovat eriteltyinä. LogParser-luokan parseInfoTrials-metodi melko pitkä, mutta tämän olitkin jo tiedostanut. loadTo-/saveToJson ovat melko pitkiä metodeja, varsinkin Jsonin lataamisen metodi, ehkä niitä saisi myös refaktoroitua pienempiin osakokonaisuuksiin.

GUI-luokka on jaoteltu hyvin ohjelman pääikkunan luomisen ja ikkunoiden toiminnallisuuden pohjilta eri luokkiin. Pääikkunan alustaminen on rakenteeltaan selkeä, jossa käytät hyvin hyödyksesi MainView-luokkaa tehden siitä paneelin.

Jos kykenet luomaan ExperimentWindow- ja MainView-luokille omat ActionListener-luokat, kyseisten luokkien rakenne keventyisi myös merkittävästi. Samoin niiden vastuulle ei jäisi sekä ikkunoiden objektien luontia ja ylläpitoa että käyttäjän syötteiden käsittelyä. MainView luokassa olet käyttänyt hyvin aliluokkaa logitiedostojen filtteröintiin. Ketterä ratkaisu.

Testiluokista sen verran lisää, että testiluokissa, joissa käytetään testattavaa dataa ja niiden tiedostopolkuja, voit halutessasi yrittää hyödyntää Javan System ("user.dir") ominaisuutta ja tallentaa testattavat tiedot projektin juureen. Tällöin testit ovat mukavasti vihreinä, vaikka projektia testailisisi muilta koneilta.

Testikattavuus vaikuttaa olevan hyvällä mallilla ja tiedostojen lukemista ja Jsonin käyttöä on testattu kattavasti, sekä huomioitu että luokat toimivat oikein myös tyhjällä datalla.

Domain-paketissa olet tehnyt kattavaa testausta ResponseMapping-luokalle. Muut paketin luokat hyödyntävätkin lähinnä settereitä ja gettereitä, joten olet fiksusti jättänyt niitä pois. Ehkä ExperimentInfo-luokan muutamia boolean metodeja ja tulosteen oikeellisuutta voit halutessasi testata, jos koet sen tarpeelliseksi.

Kaikenkaikkiaan hyvin tehty kokonaisuus. Ohjelman runko mahdollistaa hyvin suunnittelemiesi laajennusten, kuten histogramien luomisen. Toivottavasti saat sovelluksen hyötykäyttöön osaamisalallasi, tässä vaiheessa vaikuttaa erittäin lupaavalta!

Mahtis commented 7 years ago

Hei! Kiitos todella paljon kommenteista, erittäin hyvää rakentavaa palautetta! Olin itsekin juuri ajatellut, että tuota MainView'tä voisi muokata kevyemmäksi eriyttämällä AcionListenerit omiksi luokikseen, kiitos paljon näistä vinkeistä!

Testeihin liittyen muistin, että olisin jo siirtänyt testien käyttämät datat projektin juureen, mutta en näköjään sitten, pahoittelut :(. Tutustun tuohon mainitsemaasi user.diriin, ja yritän saada testit toimimaan kunnolla kaikilla laitteilla, mistä projektia ajetaan.

Koitan myös saada nuo parseri-metodit refaktoroitua pienemmeksi, kuten ehdotit.