Skip to content

Paper - Katrina Li#52

Open
likatrina7 wants to merge 2 commits into
Ada-C15:masterfrom
likatrina7:master
Open

Paper - Katrina Li#52
likatrina7 wants to merge 2 commits into
Ada-C15:masterfrom
likatrina7:master

Conversation

@likatrina7
Copy link
Copy Markdown

No description provided.

Comment thread src/App.js
}

// check winner helper function
const checkSame = (x, y, z) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great helper function!

Comment thread src/App.js
return 'o'
} else if (x === 'x' && y === 'x' && z === 'x') {
return 'x'
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If neither of these conditions are matched, the function will return a default of undefined. This will work on line 68 and beyond because undefined is falsey in JavaScript, but explicitly returning false or null makes the expected behavior more clear.

Comment thread src/App.js
// Then pass it into the squares as a callbacks
const updateSqVal = (id) => {
if (!squares[Math.floor(id/3)][id%3]['value'] && !winner ) {
const newSq = [...squares];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but if you were attempting to use it to build a copy of squares, it doesn't quite do that. Consider the following:

> const a = [[1,2,3],[4,5,6],[7,8,9]]
> console.log(a)
[ [ 1, 2, 3 ], [ 4, 5, 6 ], [ 7, 8, 9 ] ]
> const b = [...a]
> console.log(b)
[ [ 1, 2, 3 ], [ 4, 5, 6 ], [ 7, 8, 9 ] ]
> a[1][1] = "r"
'r'
> console.log(b)
[ [ 1, 2, 3 ], [ 4, 'r', 6 ], [ 7, 8, 9 ] ]
> a == b
false
> a[1] == b[1]
true

In the above example, arrays a & b are using the same inner arrays. This isn't a problem in this particular case, but I just wanted to point out that the [...array] operator is just creating a copy of the outer array, not of the inner arrays.

Comment thread src/App.js
Comment on lines +109 to +111
const newSq = generateSquares();
setSquares(newSq);
setWinner(WINNER_TBD)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great code re-use!

Comment thread src/App.js
}

const resetGame = () => {
// Complete in Wave 4
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Player 1 (X) wins and then the user clicks reset, the game will restart with Player 2 (O) as the starting player. If you want Player 1 to be the starting player for each game, this function will need to call setCurrentPlayer(PLAYER_1).

Comment thread src/App.js
Comment on lines +85 to +92
for (let row of squares) {
for (let square of row) {
sqVal.push(square.value)
}
}
if (!sqVal.includes('')) {
return 'TIE'
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jbieniosek
Copy link
Copy Markdown

Fantastic work on this project! Great code re-use and helper functions (checkSame! 😄 ). Nice work with the reset and tie extensions. This project is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants