Pine: Kris & Lia#60
Conversation
jbieniosek
left a comment
There was a problem hiding this comment.
Great work on this project Kris & Lia! This project is green.
| 036, 147, 258 vertical winners | ||
| 048, 246 diagonal winners */ | ||
|
|
||
| let winner = null; |
There was a problem hiding this comment.
There is an interesting bug in this function! The code fails the test that checks the bottom row of x's. The test setup goes as follows: 6 - x, 1 - o, 8 - x, 2 - o, 7 - x, which results in 2 o's in the top row and 3 x's in the bottom row. The if-else sequence to find the winner fails at this point, because line 94 is true! For values 3, 4 and 5, ticDict is undefined, so they are equal! As a result, winner is set to undefined.
There was a problem hiding this comment.
Oh my goodness. Thank you for figuring this one out--we were stumped!
There was a problem hiding this comment.
Haha, cool bug!!! Thanks for pointing it out :-)
| squares.forEach((row) => { | ||
| row.map((square) => { | ||
| if (square.value == '') { | ||
| square.disabled = true; |
| row.forEach((square) => { | ||
| if (square.id === squareID && square.value === '') { | ||
| square.value = turn; | ||
| ticDict[squareID] = turn; |
There was a problem hiding this comment.
This variable isn't being used correctly as a state variable. A state variable should be treated as if it is const at all times, and changes should be made through the corresponding set state function. To change a state var, make a copy, make the changes to the copy, and then call the set state function and pass in the changed copy. This can cause some issues if you want to use the new version right away, but you can bypass this by using the copied version (ie, pass the copied version to checkForWinner).
There was a problem hiding this comment.
Hi Jasmine, thank you for the feedback. Can you clarify which variable isn't being used correctly as a state variable here? Do you mean that we shouldn't be writing directly to ticDict? We should be writing to copy of ticDict and then using a set function to update it?
| } else if (ticDict[2] == ticDict[4] && ticDict[4] == ticDict[6]) { | ||
| winner = ticDict[2]; | ||
| } else if (Object.keys(ticDict).length === 9) { | ||
| winner = 'tie'; |
| winner = ticDict[2]; | ||
| } else if (ticDict[0] == ticDict[4] && ticDict[4] == ticDict[8]) { | ||
| winner = ticDict[0]; | ||
| } else if (ticDict[2] == ticDict[4] && ticDict[4] == ticDict[6]) { |
There was a problem hiding this comment.
This is a long sequence of hardcoded checks - I recommend thinking about how you could shorten this with loops.
No description provided.