-
Notifications
You must be signed in to change notification settings - Fork 18
Almog Lavi #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Almog Lavi #3
Conversation
|
|
||
| squares[index] = isXNext ? "X" : "O"; | ||
| setSquares([...squares]); | ||
| setIsXNext(!isXNext); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't modify the state object/array directly in React.
const nextSquares = [...squares];
nextSquares[index] = isXNext ? "X" : "O";
setSquares(nextSquares);
src/components/Game.tsx
Outdated
| function handleSquareClick(index: number) { | ||
| // Temporary: no gameplay logic yet | ||
| console.log("Clicked square:", index); | ||
| if(calculateWinner()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic is missing a check for whether a square is already occupied. Currently, a player can click an occupied square and overwrite its value.
src/components/Game.tsx
Outdated
| const winnerCombination = [ | ||
| [0,1,2], | ||
| [3,4,5], | ||
| [6,7,8], | ||
| [0,4,8], | ||
| [6,4,2], | ||
| [0,3,6], | ||
| [2,5,8], | ||
| [1,4,7] | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This array is static. Currently, it is being re-allocated in memory every whenever the timer ticks).
Move this outside of the component function so it's initialized only once when the module loads.
…ulate winner