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 II #2

Open
basic-ohjelmoia opened this issue Aug 26, 2016 · 0 comments
Open

Koodikatselmointi II #2

basic-ohjelmoia opened this issue Aug 26, 2016 · 0 comments

Comments

@basic-ohjelmoia
Copy link

Hei,

Tämä on vertaisarvioni Tiran harjoitustyöstäsi d3_ciph3r, repositoriolinkki: https://github.com/Antiik91/d3_ciph3r.

Kryptografiaan liittyvä aihevalintasi tuntuu oppimisen näkökulmasta paljon kunnianhimoisemmalta kuin oma aiheeni (luolakarttageneraattori roolipeliin). Kryptografiaan melko vähän perehtyneenä minun täytyy myöntää, etten tuntenut olevani lainkaan omalla mukavuusalueellani tutkiessani koodiasi! Dokumentaatiosta löytyneet lähdelinkit auttoivat kuitenkin syventämään koodin ymmärtämistä.

Latasin d3_ciph3rin lähdekoodin keskiviikkona 24.8. Tässä palautetta:

  • Vaikka käyttöliittymän kautta ei vielä päässyt käsiksi kaikkiin kryptomenetelmiin, CaesarCipherin toiminta vaikutti moitteettomalta. Myös itse luokkatoteutus vaikutti kaltaiseni kryptoummikon silmin erittäin elegantilta.
  • DES-luokka, sen pitkät kryptotaulut ja toiminnaltaan vaikeatajuiset luokat saivat minut kylmiltään luettuna täysin hämmennyksiin. Vaikka DES:n metodit olivat enimmäkseen dokumentoituja, kuvauksissa voisi kirjoittaa toimintalogiikkaa vähän seikkaperäisemmin auki.
  • Koodista löytyi jonkin verran dokumentoimattomia metodeja. Osa niistä oli pelkkiä gettereitä ja settereitä, mutta niissäkin olisi syytä kirjoittaa tarkoitus auki, kun nimeämisessä käytetään lyhenteitä (vrt. getIV, setIV).
  • Käyttöliittymäluokassasi voisi olla siistimpää refaktoroida kaikki yksittäiset execute():n alla käsiteltävät käskyt omiksi metodeikseen. saveFile()-metodissa olisi syytä käsitellä tyhjän syötteen mahdollisuus pelkkänä toiminnon peruuttamisena, nyt se johti suorituksen keskeytykseen.
  • Projektin dokumentaatio repositoriossasi vaikutti aika niukkasanaiselta, mutta dokumenttien täydentämiseen on onneksi vielä aikaa. On toki huomautettava, että dokumentointi muuttuu helposti raskaammaksi rastiksi kuin mitä sen pitäisi olla, jos kirjoitustaakkaa kasaantuu aivan projektin loppusuoralle.

Dokumentoinnin lievistä puutteista huolimatta osaat koodata lyhyitä ja ytimekkäitä metodeja -- hatunnosto sille, kuten myös kunnianhimoiselle aihevalinnallesi! Ei muuta kuin tsemppiä loppusuoralle!

Ystävällisin terveisin,

Tuomas Honkala

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