completed tic-tac-toe #8
Conversation
ROgbonna1
requested review from
Keyairra-S-Wright and
ipc103
and removed request for
ipc103
| <div className='board'> | ||
| <h3>{status}</h3> | ||
| <div className='rows'> |
There was a problem hiding this comment.
Using the "board" className is a great idea because there is now a reference to the styles. The "rows" className doesn't refer back to anything else, though. So this className could be removed...unless you plan to use it later, of course!
| @@ -1,5 +1,87 @@ | |||
| function Cells(props) { | |||
There was a problem hiding this comment.
There are four components here in App.jsx. In order to organize your code in a more modular way, I'd recommend by separating out this file by component (ie <Cells/> could go into a "Cells.jsx" file and <Board/> could go into a "Board.jsx" file, etc). By having logical chunks of code, this app will become more accessible and easier for other developers to reason about.
| <Cells click={() => clickHandler(0)} value={boardState[0]}/> | ||
| <Cells click={() => clickHandler(1)} value={boardState[1]}/> | ||
| <Cells click={() => clickHandler(2)} value={boardState[2]}/> | ||
| </div> | ||
| <div className='rows'> | ||
| <Cells click={() => clickHandler(3)} value={boardState[3]}/> | ||
| <Cells click={() => clickHandler(4)} value={boardState[4]}/> | ||
| <Cells click={() => clickHandler(5)} value={boardState[5]}/> | ||
| </div> | ||
| <div className='rows'> | ||
| <Cells click={() => clickHandler(6)} value={boardState[6]}/> | ||
| <Cells click={() => clickHandler(7)} value={boardState[7]}/> | ||
| <Cells click={() => clickHandler(8)} value={boardState[8]}/> | ||
| </div> |
There was a problem hiding this comment.
Consider simplifying this using a by mapping through the boardState in order to render the <Cells/> component without hard coding each instance.
| } | ||
|
|
||
| function Board(props) { | ||
| const [boardState, setBoardState] = React.useState(new Array(9).fill(null)); |
There was a problem hiding this comment.
This is a solid way of setting up the initial state. 👍
| const conditions = [ | ||
| [0, 1, 2], | ||
| [3, 4, 5], | ||
| [6, 7, 8], | ||
| [0, 3, 6], | ||
| [1, 4, 7], | ||
| [2, 5, 8], | ||
| [0, 4, 8], | ||
| [2, 4, 6], | ||
| ]; |
There was a problem hiding this comment.
Proactively setting up winning conditions allows helps to communicate the the programmer's decision very clearer or other developers. 👍
| if (!winner && !boardState[num]) { | ||
| boardState[num] = props.turn; | ||
| setBoardState(boardState) | ||
| props.setTurn((props.turn === 'X') ? 'O' : 'X'); |
There was a problem hiding this comment.
This is just a matter of preference: when using multiple props, the destructuring assignment can be used to help reduce code repetition.
2 participants
No description provided.