ElectricShakuhachi / tiralabra-shakugenerator

0 stars 0 forks source link

Vertaisarvio viikko 5 #1

Open jova486 opened 2 years ago

jova486 commented 2 years ago

Haettu 17.2 13.13

Hienoa!! Projekti on erittäin kiinnostava ja etenkin osana isompaa kokonaisuutta myös vaikuttaa mielekkäältä ja hyödylliseltä. Koodi on helposti ymmärrettävää ja huolella tehty dokumentointi auttaa ymmärtämään luokkien toimintaa siltä osin kun ne ovat valmiina. Arviointia haittaa että ohjelma ei nyt toimi. En osaa sanoa mistä vika johtuu. Virhe ilmoitus paikallistuu shaku_generator.py tiedostoon eli ShakuGenerator luokkaan. ”raise ValueError("Training data likely insufficient in trie").” Onko virhe siis siinä että opetusdataa ei ole vai että sitä ei saada avattua? Edelliseen liittyen Interface ja ShakuGenerator luokkien suhde jää minulle hieman epäselväksi. Mikäli Interface on käyttöliittymäluokka niin generate_output metodi ei ehkä ole ihan paras paikka sille. Minulla ei tosin auennut käyttöliittymä koska se on näemmä ohitettu tässä vaiheessa. Tämä ei varmaan ole lopullinen muoto ja siinä vaiheessa kannattaa varmaan eriyttää käyttöliittymä kokonaan suorittavasta koodista, tässä tapauksessa outputin tuottamisesta. Ehkä voisi olla välissä joku toiminnallisuudesta vastaava luokka. Mutta tästähän olikin maininta viikkoraportissasi. ShakuGenerator ja TrieTree luokkien välinen kommunikointi on mielestäni myös hieman epäselvää. Koodin kannalta voisi ehkä olla selkeämpää jos get_next olisi TrieTreen metodi. Tuntuu hieman vieraalta ajatus että toisesta luokasta kutsutaan suoraan toisen luokan muuttujia. Tässä tapauksessa TrieTree luokan muuttujan TrieNoden muutujia. Object-oriented paradigman mukaan luokka luokan tulisi tarjota metodit omien muuttujiensa käsittelyyn. Ehkä tämän voisi selkeyden nojalla miettiä uudelleen.
Muilta osin luokkien tehtävät onkin selkeästi rajattu. Testejä on paljon, näemmä 41 mutta 19 ei mene läpi. Tämä lienee johtuu siitä että on juuri tehty isoja muutoksia koodiin? Olisiko itselle helpompaa tehdä muutoksia pienempi määrä ja testata sitä mukaa kun tekee. Toiminnallisuutta ei ainakaan toistaiseksi ole niin paljoa etteikö tekstikäyttöliittymällä pärjäisi. Mikäli kuitenkin suunnittelet graafista käyttöliittymää kannattaisi nyt miettiä Interface luokan toiminta niin että voit kutsua selkeitä metodeja. Tässä on vielä niin vähän koodia että se olisi aika nopeasti tehty. Esim Tkinter tai Pyqt5 tarjoa sitten näppärän tavan käyttää sitä.

Tässä on hyvät aineksen kiinnostavaan sovellukseen (tai sen osaan mikäli niin haluat). Kuten jo tuli mainittua, koodia on helppo seurata ja rakenne näitä muutamia huomioita lukuun ottamatta on mielestäni hyvin mietitty.

ElectricShakuhachi commented 2 years ago

Hei. Kiitos monipuolisesta ja hyvästä palautteesta. Osa asioista on jo työn alla ja mainitsit myös muutamia erittäin hyviä pointteja joita en ollut tullut miettineeksi.

Koodissa tulee tuo virhe koska en ehtinyt lisätä siihen kattavaa opetusdataa, jolla järkevä generointi onnistuisi. Mutta koodissa on edellisen viikon isojen muutoksieni jälkeen muitakin ilmeentyneitä vikoja jotka täytyy korjata ennen kuin on ajettavissa normaalisti.

Graafista käyttöliittymää en ole suunnitellut kurssilla toteutettavaksi, vaan vain CLI-interfacen. Toteutan ohjelmiston siten, että se voidaan liittää toisella kurssilla luodun emo-ohjelman graafiseen toteutukseen myöhemmin omassa käytössä. Interface-nimeys luokalle joka ei ole UI on hämäävä ja siksi nimeämistä mietin varmasti jossain välissä uudelleen.

Kiitos vielä kertaalleen!