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

carmen #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

carmensalas14
Copy link
Member

No description provided.

@@ -0,0 +1,26 @@
.container{

Choose a reason for hiding this comment

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

The styles all reference the classnames you've assigned to elements. 💯

return <div>A tic-tac-toe game</div>;
return (
<div>
<h1>A tic-tac-toe game</h1>
Copy link

@Keyairra-S-Wright Keyairra-S-Wright May 2, 2020

Choose a reason for hiding this comment

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

The components in this code base are set up in a modular way. This separation of concerns keeps the code maintainable and accessible for other developers. 👍

Comment on lines +44 to +57
<div className="boardRow">
{renderSquare(0)}
{renderSquare(1)}
{renderSquare(2)}
</div>
<div className="boardRow">
{renderSquare(3)}
{renderSquare(4)}
{renderSquare(5)}
</div>
<div className="boardRow">
{renderSquare(6)}
{renderSquare(7)}
{renderSquare(8)}

Choose a reason for hiding this comment

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

Creating a renderSquare() helper function is a helpful way of reducing duplicate code. 👍 . To kick this reduction of code up a notch, consider refactoring by doing something such as mapping through the boardSquares in order to render the instances of <Square/>. This would remove the need to render each square separately.

Comment on lines +67 to +76
const winningCombos = [
[0, 1, 2],
[3, 4, 5],
[6, 7, 8],
[0, 3, 6],
[1, 4, 7],
[2, 5, 8],
[0, 4, 8],
[2, 4, 6],
];

Choose a reason for hiding this comment

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

Setting up the winning combinations makes the remainder of your code clearer. 💯

Comment on lines +3 to +4
<div className="boardSquare" onClick={props.onClick}>
{props.value}
Copy link

@Keyairra-S-Wright Keyairra-S-Wright May 3, 2020

Choose a reason for hiding this comment

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

This is just a matter of preference: when using multiple props, the destructuring assignment can be used to help reduce code repetition. This is especially helpful in components with an increased number of props.

// add next turn
squares[index] = isXPlayer ? 'X' : 'O';

// set the new state of the board squares

Choose a reason for hiding this comment

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

The inline comments can help other developer better understand the intention of this code base. Groovy. 👍

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