Skip to content

Paperr--AndreaPalacios#65

Open
dre-create wants to merge 6 commits into
Ada-C15:masterfrom
dre-create:master
Open

Paperr--AndreaPalacios#65
dre-create wants to merge 6 commits into
Ada-C15:masterfrom
dre-create:master

Conversation

@dre-create
Copy link
Copy Markdown

No description provided.

Comment thread src/App.js
const updateSquares=(id)=>{
if (winner !== null) return;

const newSquares = [...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
};


const updateStatus= () => {
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
};

/*update turn is working*/
const updateTurn = ()=> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread src/App.js
Comment on lines +119 to +123
setSquares(generateSquares());
setPlayer(PLAYER_1);
setStatus('Let\'s Play!');
setWinner(null);
setTurnCount(0)
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
Comment on lines +60 to +63
if (squares[0][i].value === squares[1][i].value &&
squares[1][i].value === squares[2][i].value &&
squares[0][i].value !== '') {
return squares[0][i].value
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 chunk of code is repeated for rows and diagonals, the only changes are which points are being checked. I recommend thinking about creating a helper function that does this comparison function to improve readability.

@jbieniosek
Copy link
Copy Markdown

Great work on this project! 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