alemati / simpleDungeonGeneratorTiralabra2019

0 stars 0 forks source link

Vertaisarviointi #1

Open Zudoku opened 5 years ago

Zudoku commented 5 years ago

Projekti kloonattu 12.4.2019, 19:22 commit: 3859d2b3c34c9dd38ec3f1fd0f48d9dc230417bd

Kloonasin projektin ja suorittamalla ohjelmaa, se toimi silmämääräisesti ihan hyvin. Testasin erilaisilla syötteillä: Järkevillä oletusarvoilla, sekä "tyhmillä / väärillä" arvoilla. Oudoilla arvoilla sovellus kaatui, mutta en koe että se on kovin vakavaa. Jos jää aikaa, niin ehkä käyttäjän inputtia voisi jotenkin validoida tai estää sovelluksen kaatuminen jos käyttäjä syöttää jotain outoa.

Kokeilin generoida suurta karttaa seuraavilla parametreilla, mutta siinä kesti todella pitkään. Tapoin sovelluksen noin 5 minuutin jälkeen. Ehkä algoritmi jää johonkin jumitilaan?

Height: 5010
Width: 3332
Minimal room size: 4
Maximal room size: 20
Room placing attemps: 200000

Ohjelman käyttäjän näkökulmasta olisi mielenkiintoista saada määäritellä huoneiden etäisyyttä tai huoneiden tiheyttä kentällä. Mielenkiintoista olisi myös, jos huoneiden väliset käytävät jotaisivat mahdollisesti moneen eri huoneeseen.


Kokeilin suorittaa yksikkötestejä, joista 2/ 4 ei mennyt läpi.

testMapShow():

expected:<######[ ###+ # ## # # #### # #    # ######] >
but was:<######[ ###+ # ## # # #### # #    # ###### ] >

testRoomAndMazeAdded():

expected:<######[ ###+ # ## # # #### # #    # ######] >
but was:<######[ ###+ # ## # # #### # #    # ###### ] >

Jostain syystä en saanut debuggattua testejä: testit eivät suoriutuneet debuggeri päällä, joten en saanut tarkemmin selvitettyä miten korjata testit. Saattaa hyvin johtua, että kehitysympäristöni ei ole oikein configuroitu. Virheestä outputista päätellen, testien virhe liittyy ylimääräiseen spaceen tai ylimääräiseen rivinvaihtoon, joka saattaa johtua kehitysympäristöstäni. Testit näyttävän menevän oikein, mutta ulostulossa ja odotetussa arvossa on joku pieni ero.


Koodin luettavuutta parantaisi koodibasen jakaminen useampaan metodiin tai luokkaan. Pidän esimerkiksi siitä, että olet jakanut logiikkaa Map luokassa eri metodeihin, joilla on yksi yksi tarkoitus. Se helpottaa koodin lukua paljon. Ehkä Map luokan addRoom metodissa luettavuutta voisi parantaa asettamalla this.squares[i][j] muuttujaan, koska sitä käytetään niin paljon. Esim

Square currentSquare = this.squares[i][j];
currentSquare.setSymbol("#");

Joidenkin if ehtojen pituus on todeella suuri, jolloin kannattaa tarkistettava ehto eriyttää omaan metodiin, jonka nimi on kuvaava.

if(i == room.getUpLeft().getxHeight() - 1 || j == room.getUpLeft().getyWidth() - 1
                            || i == (room.getUpLeft().getxHeight() + room.getHeight())
                            || j == (room.getUpLeft().getyWidth() + room.getWidth())

Square luokan status muuttuja voisi ehkä olla enum, koska kaikki siinä lienevät arvot tiedetään. Stringin käyttäminen tuossa tarkoituksessa voi olla vaarallista esim typojen takia. Enumia käytettäessä typot johtavat compile virheeseen, mutta stringeillä ne johtavat ajon aikaiseen bugiin. Enumilla voisi myös olla metodi, jolla kysyä symbolia, tällöin ei tarvittaisi omaa muuttujaa symbolille koska se ymmärtääkseni oikein riippuu aina statuksesta.

Ehdotus enum toteutuksesta:

public enum SquareStatus {
    Wall('#'), Room(' '), Open(' '), Maze(' ');

    private final char symbol;

    private SquareStatus(char symbol) {
        this.symbol = symbol;
    }

    public char getSymbol() {
        return symbol;
    }

}

Ehkä voi miettiä voisiko Square luokan muuttujat olla finaaleja, jolloin jos muutetaan jotain kohtaa kartalla, asetettaisiin kartalle siihen aina uusi Square? esimerkiksi mielestäni xHeight ja yWidth ei squarella pitäisi koskaan muuttua, joten ne voivat olla finaaleja.


Itselle jäi epäselväksi hieman iterativeFloodFill metodin toteutus. Ehkä metodin luettavuuteen voisi kiinnittää huomiota (kommentteja?, metodin pilkkomista?)


Kaikenkaikkiaan, pidän projektin aiheesta. Se osuu itselleni lähelle sydäntä: olen joskus toteuttanut itsekkin 2d peliin huoneenrakennus algoritmin, jossa oli paljon samoja piirteitä. Jatka projektin toteutusta samaan malliin!

alemati commented 5 years ago

Kiitos todella kattavasta palautteesta! Sinun ajatuksista ja vinkeistä tulee olemaan paljon apua!