Conversation
| } | ||
|
|
||
| // what are better solutions for this? | ||
| class Ids { |
There was a problem hiding this comment.
Keys 😅 My items didn't have any unique identifiers and I didn't like the warning for using index.
There was a problem hiding this comment.
This is a clever solution to this problem. To be honest, I'd have probably just stuck with the indices in this case. Using indices for keys can cause issues if the order of the array changes, but in your case those should be pretty static.
erwinssaget
left a comment
erwinssaget
left a comment
There was a problem hiding this comment.
This looks great. You did a great job, and your code is nicely formatted.
ipc103
left a comment
ipc103
left a comment
There was a problem hiding this comment.
Hey @emtes - really nice work on this one! The feature works really well, and I like how you organized the different components of the application. I left a few comments/ideas in-line, feel free to respond with any questions you might have!
| ); | ||
| } | ||
|
|
||
| function NoughtsCrosses() { |
There was a problem hiding this comment.
Just to keep things organized, I'd recommend separating each component out into a separate file. Not a big deal, but it can help make the project easier to navigate for folks coming in fresh.
| <h2>History</h2> | ||
| {history.length > 1 && ( | ||
| <p> | ||
| Hi being of the multiverse! Please go to most recent move before continuing to play. |
There was a problem hiding this comment.
👍 nice message here! One thing that might be nice is to have a separate piece of state to determine if the user is viewing history on the parent component. That way, you could disable the selecting of squares until they went back to the top of the timeline.
| } | ||
|
|
||
| // what are better solutions for this? | ||
| class Ids { |
There was a problem hiding this comment.
This is a clever solution to this problem. To be honest, I'd have probably just stuck with the indices in this case. Using indices for keys can cause issues if the order of the array changes, but in your case those should be pretty static.
| ...game, | ||
| board: newBoard, | ||
| isXNext: !isXNext, | ||
| history: [...history, [...newBoard]], |
There was a problem hiding this comment.
This definitely works.Instead of storing the entire board at each state, you could also keep track of what square was played at each turn. So, you could imagine your history looking something like [{turn: 1, played: 'X', cell: 4}, {turn: 2, played: 'O', cell: 0}] or something like that. Totally your call! In this case, I think the only tradeoff you really need to consider is readability.
| function NoughtsCrosses() { | ||
| const defaultGame = { | ||
| board: Array(9).fill(null), | ||
| isXNext: true, |
There was a problem hiding this comment.
I'd be more inclined to store the currentPlayer as a string I think. Then, you could avoid needing to use the ternary to render the correct string.
| ); | ||
| } | ||
|
|
||
| function Board({ board, handlePlay }) { |
There was a problem hiding this comment.
Nice work splitting up the concerns between these different components - makes it really easy to re-use your Board component when rendering the history.
There was a problem hiding this comment.
Could you talk more about reusability? Does it refer to reusing components for different projects? What makes components reusable? Thanks!
There was a problem hiding this comment.
Good question! Re-use probably wasn't the right word, but in this case the Board component is pretty flexible. You pass it an array of values and it will render out the game board for you. This means that, when you use the history component to change what the board should look like, it just re-renders itself. So I guess this is more about separation of concerns than re-usability. This component knows about just the right amount of stuff. Let me know if that clarifies things a little bit!
| isXNext: true, | ||
| history: [Array(9).fill(null)], | ||
| }; | ||
| const [game, setGame] = useState(defaultGame); |
There was a problem hiding this comment.
Using useState here definitely works. This component is easy to read and follow what's happening. One potential advantage to using useReducer is your reducer function could store a lot of the logic around checking for the winner and updating the board and the history. In this case, this component didn't get too big so I think this works well as-is.
| ); | ||
| } | ||
|
|
||
| function getWinner(board) { |
There was a problem hiding this comment.
In a previous comment, I mentioned splitting up the components into separate files. For functions like this, sometimes I'll create a util directory and store shared-logic in that component. Either way, I like keeping this grouped in a separate function.
3 participants
No description provided.