-
Notifications
You must be signed in to change notification settings - Fork 80
Spruce- Ivette F. #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5cb7505
c487dca
5c65394
7e58837
2cb6cfd
90b2bda
f346f1d
935093b
e42e357
dcec2d4
d23dbf6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,8 +21,7 @@ | |
| }, | ||
| "eslintConfig": { | ||
| "extends": [ | ||
| "react-app", | ||
| "react-app/jest" | ||
| "react-app" | ||
| ] | ||
| }, | ||
| "browserslist": { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,8 @@ import './App.css'; | |
|
|
||
| import Board from './components/Board'; | ||
|
|
||
| const PLAYER_1 = 'X'; | ||
| const PLAYER_2 = 'O'; | ||
| const player1 = 'X'; | ||
| const player2 = 'O'; | ||
|
|
||
| const generateSquares = () => { | ||
| const squares = []; | ||
|
|
@@ -35,31 +35,97 @@ const App = () => { | |
| // When it is clicked on. | ||
| // Then pass it into the squares as a callback | ||
|
|
||
| // How to get playerTurn to reach Square where the square state gets updated? | ||
| const [playerTurn, setPlayerTurn] = useState(player1); | ||
| const [playerTurns, setPlayerTurns] = useState(0); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This piece of state doesn't appear to be used for anything. |
||
| const [winnerState, setWinnerState] = useState(null); | ||
|
|
||
| const onClickCallback = (markedSquare) => { | ||
|
|
||
| const makeNewBoard = (squares) => { | ||
| const newBoard = [...squares]; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea to copy the existing squares state, but notice this only does a shallow copy. So we get a new outer array, but the inner arrays (the rows) and the square data objects are shared between the previous state, and the new state. This is what allows the checkForWinner call here in the click handler to appear to work. It references the squares state variable, which shouldn't appear to be updated until the next render. The fact that it is correctly seeing the winning move means that our code here is cross contaminating the previous state rather than only making a new state. |
||
| for (let row of squares) { | ||
| for (let square of row) { | ||
| if ((square.id === markedSquare.id) && (square.value === '')) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These checks are spot on. The only issue is that they result in modifying the current state and the next state, since the squares are shared between both (only the outer array was copied). |
||
| square.value = markedSquare.value; | ||
| setPlayerTurn(() => { | ||
| return ((playerTurn === player1) ? player2 : player1); | ||
| }); | ||
| setPlayerTurns((prevState) => { | ||
| return (prevState += 1); | ||
| }); | ||
| console.log('playerTurns',playerTurns); | ||
| } | ||
| } | ||
| } | ||
| return newBoard; | ||
| }; | ||
|
|
||
| setSquares(makeNewBoard(squares)); | ||
| const winner = checkForWinner(); | ||
| if (winner) { | ||
| setWinnerState(winner); | ||
| } | ||
|
Comment on lines
+65
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is only working because of the state cross contamination. But if we move the checkForWinner call to after it gets defined, and change the variable name to winnerState (and remove the useState and related stuff) the rest just works. (We'd also need to clean up the cross contamination for a full fix). |
||
| }; | ||
|
|
||
| const checkForWinner = () => { | ||
| // Complete in Wave 3 | ||
| // You will need to: | ||
| // 1. Go accross each row to see if | ||
| // 3 squares in the same row match | ||
| // i.e. same value | ||
| // 2. Go down each column to see if | ||
| // 3 squares in each column match | ||
| // 3. Go across each diagonal to see if | ||
| // all three squares have the same value. | ||
| let i = 0; | ||
| console.log('squares', squares); | ||
| // Check all the rows and columns for a winner | ||
| while (i < 3) { | ||
| if ( | ||
| squares[i][0].value === squares[i][1].value && | ||
| squares[i][2].value === squares[i][1].value && | ||
| squares[i][0].value !== '' | ||
| ) { | ||
| return squares[i][0].value; | ||
| } else if ( | ||
| squares[0][i].value === squares[1][i].value && | ||
| squares[2][i].value === squares[1][i].value && | ||
| squares[0][i].value !== '' | ||
| ) { | ||
| return squares[0][i].value; | ||
| } | ||
| i += 1; | ||
| } | ||
| // Check Top-Left to bottom-right diagonal | ||
| if ( | ||
| squares[0][0].value === squares[1][1].value && | ||
| squares[2][2].value === squares[1][1].value && | ||
| squares[1][1].value !== '' | ||
| ) { | ||
| return squares[0][0].value; | ||
| } | ||
|
|
||
| // Check Top-right to bottom-left diagonal | ||
| if ( | ||
| squares[0][2].value === squares[1][1].value && | ||
| squares[2][0].value === squares[1][1].value && | ||
| squares[1][1].value !== '' | ||
| ) { | ||
| return squares[0][2].value; | ||
| } | ||
|
|
||
| return null; | ||
| }; | ||
|
|
||
| const resetGame = () => { | ||
| // Complete in Wave 4 | ||
| setSquares(generateSquares()); | ||
| setPlayerTurn(player1); | ||
| setPlayerTurns(0); | ||
| setWinnerState(null); | ||
| }; | ||
|
Comment on lines
112
to
118
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Reseting the game is simply a matter of setting all the pieces of state back to their initial values. |
||
|
|
||
| return ( | ||
| <div className="App"> | ||
| <header className="App-header"> | ||
| <div className='App'> | ||
| <header className='App-header'> | ||
| <h1>React Tic Tac Toe</h1> | ||
| <h2>The winner is ... -- Fill in for wave 3 </h2> | ||
| <button>Reset Game</button> | ||
| <h2>Winner is {winnerState}</h2> | ||
| <button onClick={resetGame}>Reset Game</button> | ||
| </header> | ||
| <main> | ||
| <Board squares={squares} /> | ||
| <Board winnerState={winnerState} playerTurn={playerTurn} onClickCallback={onClickCallback} squares={squares} /> | ||
| </main> | ||
| </div> | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We gave bad guidance here. This should have been
since the tests were all written for lower case x and o. But you got around this by modifying your tests to use capital letters.