elad-abutbul #2
elad-abutbul #2
Conversation
| {squares.map((square, index) => { | ||
| return <Square value={square} onClick={() => onSquareClick(index)} />; | ||
| })} |
There was a problem hiding this comment.
there is no key prop here. React relies on keys to track elements between renders. Without them, React might re-render the entire list unnecessarily.
src/components/Game.tsx
Outdated
| {winner ? ( | ||
| "" | ||
| ) : ( | ||
| <h2> | ||
| time for player {isXNext ? "X" : "O"}- {timer} sec | ||
| </h2> | ||
| )} |
There was a problem hiding this comment.
Avoid repeated ternaries in JSX
{!winner && <h2>Time for player {isXNext ? "X" : "O"} - {timer} sec</h2>}
src/components/Game.tsx
Outdated
| <Board squares={squares} onSquareClick={handleSquareClick} /> | ||
| {winner ? "" : <p>Next Player: {isXNext ? "X" : "O"}</p>} |
src/components/Game.tsx
Outdated
|
|
||
| for (let i = 0; i < lines.length; i++) { | ||
| const a = lines[i][0]; | ||
| const b = lines[i][1]; | ||
| const c = lines[i][2]; |
There was a problem hiding this comment.
nitpick:
instead the existing code:
for (let i = 0; i < lines.length; i++) {
const a = lines[i][0];
const b = lines[i][1];
const c = lines[i][2];
you can use for... of loop
for (const [a, b, c] of lines) {
src/components/Game.tsx
Outdated
| <button onClick={handleReset}>reset</button> | ||
| {winner ? ( | ||
| "" | ||
| ) : ( | ||
| <button onClick={handleStop}>{stop ? "resume" : "stop"}</button> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
The conditional rendering here can be simplified.
Rendering an empty string is unnecessary and hurts readability.
Consider using && and extracting the button label logic.
const pauseButtonLabel = stop ? "resume" : "stop";
There was a problem hiding this comment.
There are several magic numbers in the component (board size, turn duration, timer interval).
Consider extracting them into named constants to improve readability and maintainability.
2 participants
No description provided.