kotommi / pakkaaja

0 stars 0 forks source link

Vertaisarviointi #1

Open joelkur opened 5 years ago

joelkur commented 5 years ago

Latasin projektin 8.4. noin klo 18.

Vaikka dokumentaatiossa ei tarvinnutkaan vielä olla käyttöohjeita eikä dokumentaatioon vertaisarvioinneissa tarvinnutkaan ottaa kantaa, voisi vertaisarvioijan näkökulmasta olla kiva jos olisi joitakin ohjeita gradlen tarvittuun versioon. Itselläni tuli siis hieman ongelmia gradlen kanssa saada ohjelmasi toimimaan (tämä voi tosin johtua ihan siitäkin, ettei gradle ole itselleni ennestään tuttu).

Testailin ohjelmaa muutamalla eri kokoisella tekstitiedostolla ja ohjelma näytti suurimmaksi osaksi toimivan ilman ongelmia. Sain parissa tapauksessa kuitenkin aiheutettua java.lang.ArrayIndexOutOfBoundsException:in Huffman-luokassa decode-metodissa rivillä 154. Tämä tapahtui suuremmilla syötteillä, kun esimerkiksi kopioin tekstitiedostoon sisältöä esim wikipediasta.

$ gradle run --args=asd.txt

> Task :run FAILED

reading file: /pakkaaja/asd.txt
reading file: /pakkaaja/asd.txt.huf
Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: 36
        at compress.encode.Huffman.decode(Huffman.java:154)
        at compress.Main.main(Main.java:35)

Välillä myös tuolla tarkistuskomennolla selvisi, ettei ohjelma ottanut joissakin tapauksissa huomioon newlinejä tekstistä.

$ diff -s asd.txt decoded.file.huf 
171c171
< Categories
---
> Categoriest
\ No newline at end of file

Kävin koodisi aika läpikotaisesti läpi, enkä rehellisesti löytänyt ollenkaan moitittavaa. Koodisi on mielestäni tosi selkeää ja helppolukuista sekä ohjelman rakenne on tosi järkevä. Lähes kaikki on selkeästi dokumentoitu ja monipuolisemmatkin asiat on kivasti kommenteilla selitetty koodissa.

Jotain häikkää kuitenkin aiemmin mainitsemani bugin perusteella tapahtuu tuossa Huffman-decode metodissa, kun viimeinen tavu käsitellään. ArrayIndexOutOfBoundsException viittaisi ainakin siihen, että length lasketaan joissakin tilanteissa väärin tuossa treeRoot.getTotalCount() kutsussa? Tämä taas voisi puolestaan viitata siihen, että joissakin tilanteissa treeRoot mahdollisesti luodaan virheellisesti Huffman-buildTree metodissa? Ainakin nämä tuli itselläni nyt äkkiseltään mieleen.

Kaiken kaikkiaan sanoisin, että ohjelmasi toimii jo nyt tosi hyvin ja vaikuttaisi olevan muutenkin lähes valmis!

kotommi commented 5 years ago

Kiitos paljon ajatuksella tehdystä arvioinnista. Voisitko littää tiedoston, jolla tuo newline-bugi tapahtuu. En saa sitä itse toistettua.

Tuo dekoodausbugi toivottavasti korjaantuu kun siirryn käyttämään otsaketta tiedostolle jossa on mukana alkuperäisen tiedoston koko tavuina

joelkur commented 5 years ago

Sori, menin jo poistamaan tuon alkuperäisen tiedoston jolla sain aikaan newline-bugin, olisi pitänyt tajuta laittaa liitteenä! Omituisesti en saa enää itsekkään tuota bugia toistettua vaikka kuinka yritän. Isot pahoittelut jos olinkin vain itse onnistunut mokaamaan jotenkin!