Koodikatselmointi

[taapp]

Latasin projektin zippinä päivänä 2.5.2021 klo 13:36.

Yleisilmeeltään koodi vaikuttaa selkeältä ja itse peli on tyylikkään näköinen.

Seuraavassa joitakin huomioita ja mahdollisia parannusehdotuksia.

main.py:

• En itse tehnyt Pygame-sovellusta, mutta main-funktion rakenne näyttää samanlaiselta kuin Sokoban-esimerkissä, joten oletan sen olevan hyvä.

board.py:

• Koodissa on mukana kommentit "# Luo cards taulukon, johon se lisää Card pareja 9kpl." ja "# Luo 3x6 Ruudukon.". Mielestäni näissä on sellainen ongelma, että ruudukon koko ymmärtääkseni riippuu ROWS- ja COLUMNS -argumenteista, joiden arvot alunperin tulevat main.py:ssä kovakoodatuista arvoista. Tämä aiheuttaa sen, että jos jossakin vaiheessa ruudukon kokoa muutetaan, niin kyseiset kommentit eivät enää pidä paikkaansa. Lisäksi voi olla myös hankalaa huomata, että kommentteja pitäisi päivittää. Esimerkki kestävämmästä vaihtehdosta voisi olla "# Luo ROWSxCOLUMNS-kokoisen Ruudukon."

• Luullakseni ruudukon luovaa silmukkaa voisi yksinkertaistaa jossain määrin siinä mielessä, että toisteisia rivejä voisi poistaa. Esimerkiksi ainakin rivit (jos luin oikein) "for j in range(self.columns):", "self.cards[k].set_location(self.card_grid[i][j])" ja "k += 1" ajetaan jokaisessa silmukan haarassa, joten niitä varmaan ei tarvitsisi toistaa, vaan ne voisivat esiintyä vain yhden kerran.

• Pienenä vinkkinä, silmukassa, jossa luodaan Card-luokan oliot, annetaan korteille myös id:t. Näiden kirjanpitämiseen käytetään kahta koodiriviä ("id = 0" ja "id += 1"). Nämä kaksi riviä voitaisiin kuitenkin poistaa kirjoittamalla silmukan aloittava rivi (rivi 21) seuraavalla tavalla: "for id,card in enumerate(self.file_loader.get_cards()):" Eli siis enumerate:n avulla saadaan suoraan määriteltyä silmukkaindeksi, joka alkaa nollasta ja kasvaa yhdellä joka kierroksella.

renderer.py:

• Koodissa esiintyy useita kertoja tuple (255, 255, 255). Tuplesta voisi mahdollisesti tehdä vakiomuuttujan, jota sitten voisi käyttää toistuvasti.

testejä määrittelevät tiedostot kuten card_test.py:

• Testien nimissä on myös suomea esim. card_test.py:ssä on "test_konstruktori_asettaa_id_oikein". Luulisin olevan parempi, että nimet olisivat kokonaan englanniksi. Tämän asian muuttaminen tässä vaiheessa ei olisi kovin suuri vaiva.

Hyvää työtä!

[VoxBorealis]

Erinomaisia huomioita!

Itselle ei ollutkaan tuo Enumerate ennestään tuttu. Muutin sovellusta ehdotuksiesi mukaan, kiitos!