Sarah J. Olivas - Spruce#66
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!
| onClick={() => { | ||
| props.onClickCallback(props.id); | ||
| }} |
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 PLAYER_1 = 'X'; | ||
| const PLAYER_2 = 'O'; | ||
| const PLAYER_1 = 'x'; | ||
| const PLAYER_2 = 'o'; |
There was a problem hiding this comment.
We gave bad guidance here. This should have been
const PLAYER_1 = 'x';
const PLAYER_2 = 'o';since the tests were all written for lower case x and o. But it looks like you took care of this on your own!
| // resets squares state and player state. | ||
| const resetGame = () => { | ||
| // Complete in Wave 4 | ||
| setSquares(generateSquares()); | ||
| setPlayer(PLAYER_1); | ||
| }; |
There was a problem hiding this comment.
Yep. Resetting the game essentially comes down setting the pieces of state back to their initial values.
| const statusLine = () => { | ||
| if (winner) { | ||
| return `Winner is ${winner}`; | ||
| } | ||
| return `${player}'s turn`; | ||
| }; |
There was a problem hiding this comment.
Nice helper to update the status line for the player turn or game winner.
| // doesn't allow anymore moves after a win. | ||
| if (winner) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
We can check the winner due to closure semantics (winner is defined in the outer execution context).
| const updateSquares = squares.map((row) => | ||
| row.map((pos) => { | ||
| if (pos.id !== id || pos.value !== '') { | ||
| return pos; | ||
| } | ||
|
|
||
| // valid move if passes all checks | ||
| move = true; | ||
|
|
||
| // create new array that includes played square(s) | ||
| const playedSquare = { ...pos, value: player }; | ||
| return playedSquare; | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Good use of double mapping and spreading the player square so that when a move is made, the new state is no longer connected to the old state (avoid state cross contamination).
3 participants
No description provided.