reine93 / Hangman-in-React

Simple hangman game coded using ReactJS library
0 stars 0 forks source link

Prvi review #1

Open stanko-ingemark opened 10 months ago

stanko-ingemark commented 10 months ago

Ovo je dosta dobro, solidno se ponaša i dosta je logično strukturirano. Nekoliko komentara

1. Općenito

  1. u konzoli dobivam poruku "Received true for a non-boolean attribute danger". Ovo ne utjeće na igru, ali pokušavamo se riješiti suvišnih poruka u konzoli da nam ne stvaraju buku
  2. kod uspješnog završetka igre se šalje više requestova na GET /highscores
  3. kada prvi puta počinjemo igru (nakon unosa imena, ne reset), pošalju se dva requesta na GET /random

2. src/screens/GameScreen.js

  1. 'timeUpdated' - nije mi baš intuitivno čemu ovo služi i zašto se zove kako se zove.. izgleda kao da prati kada je igra aktivna ('isPlaying')?
  2. jedini efekt koji imamo od praćenja tog stanja je da se pošalju highScoreovi kada igra završi ili još nešto? Možemo li pojednostaviti tu logiku? Može li taj useEffect ovisiti samo o gameWon (linija #31)

3. src/components/game/UserInfo.js

  1. Ovo nekada služi za input imena, a nekada pokazuje broj grešaka..možda je malo neintuitivno što se tiče naziva (zašto je broj errora userInfo, a ne iskorištena slova npr.?), ali veći "problem" je s nejasnom odgovornošću - u osnovi imamo komponente koje služe za prikaz nečega (npr. HangmanDraw), i komponente koje enkapsuliraju nekakvu logiku (dohvat podataka, odlučivanje koje podkomponente će se prikazati - npr. HangmanFrame) - nije jasno u koju kategoriju ova komponenta spada

4. src/components/game/GameTimer.js

  1. Kod računanja protoka vremena, nije dobro oslanjati se na interval ili timeout, jer nisu egzaktni (izvrše se kada dođu na red, što ovisi o ostatku aplikacije). Za mjerenje protoka vremena se bolje oslanjati na apsolutno vrijeme (najbanalnije const startTime = Date.now(); ...; elapsedTime = Date.now() - elapsedTime).. U redu je da interval koristimo za samu provjeru (kada updateamo 'elapsedTime')
  2. zašto proslijeđujemo onGameReset u ovu komponentu (ona ga poziva na promjenu reseta koju opet dobijemo izvana.. zašto GameScreen odmah ne zove onGameReset na reset?

5. src/components/game/QuoteGuesser.js

  1. Ovdje nam je dio logike same igre.. jel bi taj dio mogao isto ići u redux?
reine93 commented 10 months ago

U konzoli dobivam poruku "Received true for a non-boolean attribute danger". Ovo ne utjeće na igru, ali pokušavamo se riješiti suvišnih poruka u konzoli da nam ne stvaraju buku

Ovo je popravljeno

kod uspješnog završetka igre se šalje više requestova na GET /highscores kada prvi puta počinjemo igru (nakon unosa imena, ne reset), pošalju se dva requesta na GET /random

App mi je bio wrappan u <StrictMode>, sada sam ga maknula pa imam po jedan GET na /highscores i /random

2. src/screens/GameScreen.js

  1. 'timeUpdated' - nije mi baš intuitivno čemu ovo služi i zašto se zove kako se zove.. izgleda kao da prati kada je igra aktivna ('isPlaying')?
  2. jedini efekt koji imamo od praćenja tog stanja je da se pošalju highScoreovi kada igra završi ili još nešto? Možemo li pojednostaviti tu logiku? Može li taj useEffect ovisiti samo o gameWon (linija #31)

Jednom kada je igra zavrsena, u gameTimeru postoji effect koji updejta state.timeElapsed u reduxu sa lokalnim elapsedTime stateom


      if (gameWon) {
        dispatch(addTime(elapsedTime));
        onGameEnd();
      }

Nakon toga zove onGameEnd koji je zapravo handleGameEnd u GameScreenu, koji javlja da je state.timeElapsed updejtan i da se onda moze poslati highScore sa svim potrebnim podatcima.

Dok sam ovo razvijala, prvotno mi se timeElapsed updejtao direktno u Reduxu preko dispatcha svake sekunde dok je igra aktivna. Problem je bio u tome sto bi mi se onda cijela Hangman komponenta rerenderala svake sekunde kada bi igra bila pokrenuta i kada bi timer krenuo

Onda mi je rjesenje bilo da game timer zasebno ima svoj lokalni elapsedTime state koji updejta svake sekunde(tako da se samo gameTimer komponenta rerenderira svake sekunde), pa u trenutku kada je igra zavrsena tj kada je gameWon, on updejta redux state.timeElapsed sa lokalnim elapsedTime.

Kod useEffecta koji se bavi slanjem highScorea u GameScreenu


  useEffect(() => {
    if (gameWon && timeUpdated) { //once game won and elapsed time from timer has been updated, send highscore and set timeUpdated back to false 
      sendHighscore(quoteId, quoteLen, uniqueChars, playerName, errorNum, timeElapsed);
      setTimeUpdated(false)
    }
  }, [timeUpdated]);

Ako samo stavim da je if(gameWon), on ce poslati highscore prije nego sto se state.timeElapsed stigne azurirati. Ako stavim da je if (gameWon && timeUpdated) ali da useEffect ovisi samo o [gameWon], onda ce timeUpdated se nece stici prebaciti u true, if (gameWon && timeUpdated) ce biti false pa se sendHighscore nece pokrenuti.

Nisam sigurna koliko je ovo najbolje rjesenje, najbolje za sada sto sam uspjela smisliti….

3. src/components/game/UserInfo.js

  1. Ovo nekada služi za input imena, a nekada pokazuje broj grešaka..možda je malo neintuitivno što se tiče naziva (zašto je broj errora userInfo, a ne iskorištena slova npr.?), ali veći "problem" je s nejasnom odgovornošću - u osnovi imamo komponente koje služe za prikaz nečega (npr. HangmanDraw), i komponente koje enkapsuliraju nekakvu logiku (dohvat podataka, odlučivanje koje podkomponente će se prikazati - npr. HangmanFrame) - nije jasno u koju kategoriju ova komponenta spada

Moja ideja za UserInfo je bila samo da prikazuje ime igraca(koje moze editirati) i broj gresaka koje je igrac napravio

Dok je prikaz iskoristenih slova stavljen kod QuoteGuesser input fielda, jer sam gledala tako da se prikaz iskoristenih slova prikazuje iznad input fielda u koje je igrac upisivao ta slova, koji su dio QuoteGuesser komponente

4. src/components/game/GameTimer.js

  1. Kod računanja protoka vremena, nije dobro oslanjati se na interval ili timeout, jer nisu egzaktni (izvrše se kada dođu na red, što ovisi o ostatku aplikacije). Za mjerenje protoka vremena se bolje oslanjati na apsolutno vrijeme (najbanalnije const startTime = Date.now(); ...; elapsedTime = Date.now() - elapsedTime).. U redu je da interval koristimo za samu provjeru (kada updateamo 'elapsedTime')

Ovo je popravljeno, GameTimer komponenta je refaktorirana i sada se oslanja na apsolutno vrijeme. Dio logike prebacen u redux

  1. zašto proslijeđujemo onGameReset u ovu komponentu (ona ga poziva na promjenu reseta koju opet dobijemo izvana.. zašto GameScreen odmah ne zove onGameReset na reset?

Ovo je refaktorirano, ali slicno kao kod issuea #2:

  1. u gameSlice initial state je dodan gameReset (postavljen na false) da prati ako je igra resetirana
  2. u resetGameState reducer je dodan state.gameReset = true
  3. Novi reducer (completeGameReset) state.gameReset stavlja na false;
  4. Jednom kad se stisne reset button, dispatcha se gameReset a onda u gameTimeru:
      if (gameReset) {
        setElapsedTime(0); 
        dispatch(completeGameReset()); //set game reset back to false as timer has been reset
      }

-Ovo sam napravila tako jer game timer mi se ne bi inace pravilno resetirao dok nisam napravila zasebnu funkciju koja gameReset vraca na false. Moja prva ideja je bila da u resetGameState reducer stavim

state.gameReset = true
//…ostatak koda…//
state.gameReset = false

ali onda gameTimer ne bi registrirao promjenu sa gameReset stateom jer bi reducer opet vratio false

Druga ideja je bila da u handleReset stavim resetGameState reducer ovako


  const handleReset = () => {
    dispatch(resetGameState());
    handleFetchQuote();
dispatch(completeGameReset())
  };

ali opet, handleReset bi vratio state.gameReset = false i timer ne registrira promjenu state te vrijeme se ne resetira

onGameReset je potpuno maknut

5. src/components/game/QuoteGuesser.js

  1. Ovdje nam je dio logike same igre.. jel bi taj dio mogao isto ići u redux?

-handleGuess je sada prebacen u Redux