Vertaisarviointi - Githubissues

Projekti kloonattu 7.10.2022 klo 19.02.

Moikka!

Kiinnostava aihe, vaikka en tunne peliä entuudestaan. Olen aika pihalla hyvistä React- ja frontend-käytännöistä, joten päätin keskittyä palautteessa lähinnä pelin logiikkaa ja tekoälyä toteuttavaan koodiin.

Dokumentaatio

Projektin dokumentaatio on mielestäni kattavaa, perusteellista ja yksityiskohtaista, mutta samalla riittävän ytimekästä. Keskeneräisten osien täydentämisen lisäksi en lähtisi muuttamaan mitään :)

Projektin rakenne ja koodin laatu

Projektin rakenne on järkevä ja johdonmukainen. Kuten ylempänä mainitsin, en ymmärrä React-projektien konventioista mitään, mutta pelin logiikkaa toteuttava koodi on jaettu osiin järkevästi. Yksittäiset tiedostot ovat mielestäni sopivan kokoisia, enkä näe tarvetta jakaa koodia useampiin alikansioihin.

Koodi on mielestäni selkeää ja kattavasti kommentoitua. Kaikki kommentit eivät tosin ole täysin ajantasaisia (esim. simulate- ja alphabeta-funktioiden parametrit eivät vastaa kommenteissa listattuja).

Toiminnallisuus

Sovelluksen käyttöliittymä on todella hieno ja helppokäyttöinen. Tekoäly toimii ainakin omalla MacBookillani todella nopeasti enkä huomannut pelatessa minkäänlaista viivettä. Toisaalta tekoäly on aika helppo voittaa. Nämä molemmat varmaankin selittyvät sillä, että tekoäly näyttää laskevan siirrot ainoastaan 4 siirron päähän.

Testit

Testit löytyvät omasta kansiostaan, mikä on mielestäni ihan hyvä (ja mahdollisesti käytetyn testauskirjaston vaatima) valinta. Yksittäiset testit ovat selkeitä ja niistä käy hyvin ilmi testattava asia. Kaipaisin ehkä vähän lisää testejä erityisesti pienemmille funktioille, mutta varmasti nykyisilläkin pärjää.

Parannusehdotuksia

Mielestäni utils on hieman hämäävä kansion nimi koodille, joka toteuttaa pelin säännöt ja tekoälyn. Kuvaavampi nimi voisi olla esim. game tai battle-sheep.
Funktion nextPlayerIndex voisi toteuttaa yksinkertaisemmin esim. (index + 1) % 2.
Funktion AI_selectStart nimi ei tällä hetkellä noudata projektissa muuten käytettyä camelCase-nimeämiskonventiota. Sen voisi uudelleennimetä aiSelectStart.
Koodissa pelilautaa (Board) ja pelilaudan kokoa kuvaavia muuttujia pyöritellään erillään toisistaan. Mielestäni voisi olla perusteltua sisällyttää kumpikin tieto saman rakenteen sisään, etenkin kun laudan koko on käsittääkseni pääteltävissä laudan sisällöstä.
game.ts-tiedoston funktiot palauttavat aina kopion laudasta. Luulisin, että suorituskyvyn parantamiseksi voisi olla järkevää muokata lautaa kopionnin sijaan esimerkiksi tässä https://github.com/ronituohino/tiralabra/blob/738f9e5419bdcf5273c5433fbce2fc8dbaab2c2c/src/utils/ai.ts#L137
Erityisesti game.ts-tiedoston koodi näyttää siltä, että funktioiden paketoimisesta luokan taakse voisi olla hyötyä. En tosin ole varma onko näin lainkaan tapana tehdä ts-projekteissa. Se saattaisi myös hankaloittaa koodin käyttöä React-komponenteista käsin.

Mahdollisia bugeja/outouksia

Kun pelaa pelin loppuun, painaa back to config ja aloittaa uuden pelin, niin kaatuu:
```
Uncaught RangeError: Maximum call stack size exceeded
```
Yksittäistä lammasta ei voi siirtää, mutta esim. 5 lampaan kasasta voi siirtää kaikki lampaat. Tämä saattaa toki kuulua pelin sääntöihin, mutta vaikuttaa ainakin äkkiseltään epäjohdonmukaiselta.

ronituohino / battle-sheep

Vertaisarviointi #2