Vertaisarvostelu

[hjuha]

Arviointi tehty hetkellä 4.10.2018 kello 16:38 ladatulle versiolle

Rakenne

Koodin vaikuttaa rakenteeltaan hyvältä ja järkevältä ja siinä käytetään tietorakenteiden perintää siististi.

Ongelmia

1.
Luokan PrimObjectHeap swap-funktio heittää ilmeisesti aina kutsuttaessa virheen. Tämä kuitenkin otetaan kiinni, mutta tästä seuraa satunnaisesti, että ruudukosta tulee epäyhtenäinen. Virheen aiheuttaa se, että luokassa MinHeap<T> luodaan taulukko tyyppiä Comparable[], joka tyyppimuunnetaan tyyppiin T[]. Java ei heitä tässä vaiheessa virhettä, mutta käytöstä voi seurata ongelmia näin tehtynä [1]. Ratkaisuksi ongelmaan toimisi ilmeisesti Javan Reflection API:n käyttäminen funktiolla Array.newInstance [2].

2.
Luokan MinHeap yksikkötestit vaikuttavat testaavan nyt vain erikoistapauksia: joko aidosti kasvavaa syötettä tai aidosti vähenevää syötettä. Varmuuden vuoksi olisi hyvä lisätä yleisempi tapaus testeihin. Kokeilin tällaisen testin lisäystä, ja sen jälkeen testit menivät läpi, joten luokka näyttäisi toimivan oikein. Sama pätee luokan PrimObjectHeap yksikkötesteihin.

3.
Luokassa RoomFactory on konstruktori, joka ottaa parametrikseen Random-tyypin olion. Tätä arvoa ei kuitenkaan käytetä mihinkään, sillä konstruktorin sisällä luodaankin uusi sellainen new-avainsanalla. Oletettavasti ideana on mahdollistaa tietyllä siemenluvulla alustetun Random:n käyttö?

4.
Luokan PrimObjectHeap funktio update ei toimi oikein. Tämän takia ohjelman generoimat luolastot eivät aina noudata pienintä virittävää puuta, kuten seuraavasta tulostuksesta näkee:

Puu olisi kevyempi yhdistämällä kaksi ylintä huonetta. Ongelman ratkaisuun auttanee seuraavan yksikkötestin lisääminen luokalle PrimObjectHeap:

@Test
public void updateTest() {
    PrimObjectHeap heap = new PrimObjectHeap(20);
    PrimObject obj = createPrimObject(1, 0);
    PrimObject obj2 = createPrimObject(2, 1);
    heap.insert(obj);
    heap.insert(obj2);
    obj2.setValue(0);
    heap.update(obj2);
    assertEquals(heap.deleteMin().getValue(), 0, 0.1);
    assertEquals(heap.deleteMin().getValue(), 1, 0.1);
}

Nykyisin kyseinen testi palauttaa ensin arvon 1 ja vasta sen jälkeen arvon 0.

Yleisesti ottaen työssä on lähes kaikki hyvän tavan mukaista ja sen tuottamat luolastot kiinnostavia! :)