Skip to content
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

Cedar - Kit/Laurel #59

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Cedar - Kit/Laurel #59

wants to merge 5 commits into from

Conversation

KitSutliff
Copy link

tic-tac-toe submission

Copy link

@alope107 alope107 left a comment

Choose a reason for hiding this comment

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

Nice job! You've got some great logic here. There are some bugs that can arise after the game has been won, but the overall functionality is good! This is a nice showcase of your knowledge of React!

Comment on lines +30 to +32
const [squares, setSquares] = useState(generateSquares());
const[winner, setWinner] = useState(null);
const [currentPlayer, setCurrentPlayer] = useState(PLAYER_1);

Choose a reason for hiding this comment

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

Nice use of state!

Comment on lines +34 to +35
let i = 0;
while (i < 3) {

Choose a reason for hiding this comment

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

Consider using a for loop here instead.

Comment on lines +76 to +88
let newBoard = squares.map((square)=>{
for (let property of square){
if (property.id === id && property.value === ''){
if (currentPlayer === PLAYER_1){
property.value = PLAYER_1;

} else if (currentPlayer === PLAYER_2){
property.value = PLAYER_2;
}
}
}
return square;
});

Choose a reason for hiding this comment

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

Nice map!

header = <h2>Winner is {winner}</h2>;
} else {
header = <h2>The Current Player is {currentPlayer}</h2>;
boardCallback = onClickCallback;

Choose a reason for hiding this comment

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

Because this callback is only set if there's not a winner weird bugs can arise. For example, if someone wins the game, and then a square is clicked again, your whole React app will crash (not even the reset button will work anymore). Consider ways you can resolve this by always passing a callback but having it do different things if the game is won or not.

return (
<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>
{header}

Choose a reason for hiding this comment

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

Nice use of state!

<Square value={square.value}
id={square.id}
onClickCallback={onClickCallback}
key={square.id}

Choose a reason for hiding this comment

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

Good job remembering to use key!


}

const singleSquares = [].concat(...squares);

Choose a reason for hiding this comment

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

This can be done slightly simpler with const singleSquares = [...squares];

// Component to alert a parent
// component when it's clicked on.

const hardestButtonToButton = () => {

Choose a reason for hiding this comment

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

Fun function name 😛

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.

3 participants