Skip to content

Conversation

@Lavi2910
Copy link

…ulate winner

@ShirYahav ShirYahav requested a review from OfekSagiv December 17, 2025 08:51
Comment on lines 40 to 43

squares[index] = isXNext ? "X" : "O";
setSquares([...squares]);
setIsXNext(!isXNext);

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);

Comment on lines 10 to 38
function handleSquareClick(index: number) {
// Temporary: no gameplay logic yet
console.log("Clicked square:", index);
if(calculateWinner())

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.

Comment on lines 10 to 19
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]
]

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants