Sandra Caballero- Spruce#73
Conversation
anselrognlie
left a comment
anselrognlie
left a comment
There was a problem hiding this comment.
Nice job!
All tests (including the optional reset tests) are passing.
Nice job using the checkForWinner in the flow of the component function rather than in immediate response to a click. Since the state value isn't updated until the next render, we can't correctly use the function during the click handler (unless we accidentally cross contaminate our new state with the current state). But the way you called checkForWinner respects the state properly.
Well done!
| {props.value} | ||
| </button> | ||
| } | ||
| return <button onClick={()=> props.onClickCallback(props.id)} className='square'>{props.value}</button>; |
There was a problem hiding this comment.
Nice inline handler to call our click callback when the button was clicked. Notice that we can't simply use props.onClickCallback directly as the onClick handler, since we need to pass the id of the clicked square to the callback. If we had used it as the onClick directly, then the event details provided by the browser would have been passed in as the first parameter rather than the id.
| // const [winner, setWinner] = useState(null); | ||
| // const [gameEnd, setGameEnd] = useState(false); |
There was a problem hiding this comment.
Exactly. We don't need these pieces of state, since the winner can be calculated from the board, and whether the game is over can be determined from the winner. No additional state needed!
| // Wave 2 | ||
| const squareClicked =(id)=>{ | ||
| let didPlay = false | ||
| if (winner !== null) {return} |
There was a problem hiding this comment.
👍
If winner has a value, then the game is over, and we shouldn't process any more board clicks. So we can simply return to exit the function without modifying any state.
| const squareClicked =(id)=>{ | ||
| let didPlay = false | ||
| if (winner !== null) {return} | ||
| const newState = squares.map(row =>row.map(pos =>{ |
There was a problem hiding this comment.
👍
We map both the outer and inner list. If we end up making a play, we don't want to modify the existing state directly. We want to make a new state, and notify React about it through setSquares.
| if (pos.value !== '') {return pos;} | ||
| //we know the id matches | ||
| didPlay = true | ||
| const newPos = {...pos} |
There was a problem hiding this comment.
👍
Here, we spread the square position information from the existing state to copy it. When we set the new value, we only want to modify the new state data, not the existing state.
| newPos.value = currentPlayer | ||
| return newPos | ||
| })) | ||
| if (didPlay) { |
There was a problem hiding this comment.
👍
Only update the state if the player ended up making a valid move. We are able to declare didPlay in our click handler, but modify it within the anonymous functions passed to map, due to closure semantics. didPlay is defined in the execution context of the map functions, so they can see and modify it. This lets us detect whether a move occurred here.
| const resetGame = () => { | ||
| // Complete in Wave 4 | ||
| setSquares(generateSquares()); | ||
| setCurrentPlayer(PLAYER_1); | ||
| // setWinner(null); | ||
| // setGameEnd(false); | ||
| }; |
There was a problem hiding this comment.
👍
Resetting the game essentially comes down to setting the state back to its initial values.
| <h1>React Tic Tac Toe</h1> | ||
| <h2>The winner is ... -- Fill in for wave 3 </h2> | ||
| <button>Reset Game</button> | ||
| <h2>Winner is {winner} </h2> |
There was a problem hiding this comment.
It would be nice to use conditional rendering here to avoid displaying the winner message until after the game is over. Alternatively, we could display the current player's turn here until the game has ended. We could also think about how to detect whether the game ended in a tie, and display a message to that effect.
3 participants
No description provided.