nameisxi / graph-layout-generator

0 stars 0 forks source link

Vertaisarviointi 2 #2

Open KyperCT opened 2 years ago

KyperCT commented 2 years ago

Perustuu 24.2. ladattuun versioon

Projekti yleisesti vaikuttaa hyvältä. Koodi on selkeää, ja hyvin kommentoitua. Sovelluksen rakenne on erinomainen.

Testien ajo ei onnistunut ohjeistuksen mukaisella tavalla koulun koneella (aiheutti import ongelmia), joten ajoin testit pytestillä, jolla ne mene läpi. Tällä tavalla ajettuna jokin testeistä ei mennyt läpi. Kun luin läpi testejä, ihmettelin, miksi niitä oli vain muutama, vaikka kuitenkin assertioita on useita. Omalla kohdalla ainakin testit olisi helpompi lukea läpi, jos jokainen assertio olisi oma metodinsa, ja olisi helpompi tietää, mikä ei mennyt läpi. Ongelmahan voi hyvinkin olla konffissa, mutta kun en juuri tätä ohjelmistoa niin tarkkaan ymmärrä niin tuollaisilla suurtesteillä on haastavaa tietää onko ohjelmassa jokin aito vika vai olenko tehnyt konfiguraatiovirheen.

Sen verran mitä niistä ymmärrän, testit vaikuttavat sisällöllisesti hyviltä, ongelmat ovat juuri tuossa rakenteessa. Myös, testiluokissa oli globaaleja muuttujia, voisi olla parempi siirtää ne "Set up" metodiin.

Mutta tosiaan muuten projekti on erinomainen, ja vaikuttaa siltä että tiedät mitä teet. Parasta onnea lopputyön kanssa!

nameisxi commented 2 years ago

Kiitos palautteesta!

Testien ajo ei onnistunut ohjeistuksen mukaisella tavalla koulun koneella (aiheutti import ongelmia), joten ajoin testit pytestillä, jolla ne mene läpi. Tällä tavalla ajettuna jokin testeistä ei mennyt läpi.

En pystynyt uudelleen toistamaan ongelmia MacOS:llä ja Ubuntulla. Mitä Python versiota ja käyttöjärjestelmää käytit? Omalla puolellani kaikki testit menevät myös läpi ja ne eivät onnistu vain jos käytät Python 2:sta, jolloin jokainen get_coordinates() metodin testeistä näyttäisi palauttavan saman suuruisen, mutta eri tyyppisen float arvon.

Omalla kohdalla ainakin testit olisi helpompi lukea läpi, jos jokainen assertio olisi oma metodinsa, ja olisi helpompi tietää, mikä ei mennyt läpi.

Kiitos palutteesta. Käytin tätä tyyliä, koska Pythonin unittest standardikirjaston dokumentointi suositteli testaamaan jokaista metodia yhdellä testillä, joka sisältää useamman assertion.

Myös, testiluokissa oli globaaleja muuttujia, voisi olla parempi siirtää ne "Set up" metodiin.

Yritin käyttää konstruktoria, mutta niitä ei unittestin perimissä luokissa voi käyttää ja näin vastaavia globaalien muuttujien esimerkkejä unittest dokumentaatiossa ja StackOverflowssa. Käyttäisitkö itse siis erillistä metodia, jota testimetodit kutsuvat?

Kiitos vielä palautteesta ja onnea lopputyön pariin!