Conversation
ipc103
left a comment
ipc103
left a comment
There was a problem hiding this comment.
Hey @Devonte202 - really nice work with this Tic Tac Toe game! I like your choice to use useReducer as well as the context for the score. I left a bunch of comments inline - feel free to respond if you have any questions or thoughts!
|
|
||
| const [step, setStep] = useState(0) | ||
|
|
||
| const deleteParadox = (clickedButton) => { |
There was a problem hiding this comment.
I love this function name, but I'm a little bit confused about what this function is doing. Is this erasing part of the history when you travel back?
| import TimeMachine from './TimeMachine.js' | ||
| import { ScoreContext } from '../contexts/ScoreContext.js' | ||
|
|
||
| const board = [null,null,null,null,null,null,null,null,null] |
There was a problem hiding this comment.
You can using Array(9).fill(null) to do this so you don't have to hard-code it out.
|
|
||
| const board = [null,null,null,null,null,null,null,null,null] | ||
| let turn = 0 | ||
| let timeSteps = [] |
There was a problem hiding this comment.
To respond to your global variable comment - I'd probably just nest these inside of the component :-) Another option would be to create a custom hook called useBoard or something that could hold these initial values, plus the useReducer call below.
|
|
||
|
|
||
| const Board = () => { | ||
| const [score, setScore] = useContext(ScoreContext) |
There was a problem hiding this comment.
👍 I really like how you separate the score from the other board state via the context. Keeps your concerns nice and should scale well.
| return state | ||
| } | ||
| } | ||
| const [state, dispatch] = useReducer(boardReducer, board) |
There was a problem hiding this comment.
👍 👍 for useReducer in this case - I think this is a really good choice for storing the state. This lets you handle the time travel relatively easily.
There was a problem hiding this comment.
You could probably even let your timeSteps state be a part of this as well. That way, you could handle that with separate actions instead of with useEffect. You could even check for the winner each time the board is updated here in the reducer.
| @@ -0,0 +1,9 @@ | |||
| import React from 'react' | |||
|
|
|||
| const PlayerTwo = () => { | |||
There was a problem hiding this comment.
This is kind of cool - means that you can change the player value if needed relatively easily.
|
|
||
| useEffect(() => { | ||
| timeSteps.push(state) | ||
| turn += 1 |
There was a problem hiding this comment.
Instead of using a global variable for your turn, I'd store it as a piece of application state. That way, you can update it when you update the other moves and when you go back in time, you can reset to the correct player's turn.
| <div className="boardArea"> | ||
| <div className="board"> | ||
| {state.map((box, index) => { | ||
| if(box === "X"){ |
There was a problem hiding this comment.
Looking at this again, it might be easier to just render 'X' or 'O' directly. I think having the separate Player components probably adds some un-needed complexity.
| } | ||
| else { | ||
| return ( | ||
| <div key={index} className="box" id={index} onClick={() =>dispatch({type: 'move', index: index})}></div> |
There was a problem hiding this comment.
To make sure that the square can only be selected once, you could have this onClick function check to see if it's an occupied square or not before firing the dispatch.
3 participants
Known bugs
Known areas for optimization