Accelerate - RA

[rishallen]

No description provided.

[anselrognlie]

Almost there!

It looks like you had some good ideas on how to approach all the required parts of the project. There were just a few critical spots where that last bit of connection was missing. Take a look at my feedback for some ideas on how to get those connections made.

This project has a lot to keep in mind, such as the shape of the squares data (some places it's 2D, others it's 1D), event handler callbacks with expected parameters and closure semantics, and introducing additional state. So try experimenting with those concepts in your own projects/sandboxes to get more of a feel for them.

Keep at it!

[anselrognlie]

Note, there was a wave 2 test in Square.test.js that needed to be unskipped. The test passed, but for slightly unexpected reasons. See my notes about the event handling on the square button for more details.

[anselrognlie]

Your updateSquare function that you pass in for the onClickCallback expects two params: the ID, and the value. The value isn't strictly necessary, since the current square value can be looked up in App's square data, but your implementation uses the passed value to quickly decide whether any processing is required (if the current value isn't blank, skip the click), so that should get passed into the function.

Without this, click handling was non-functional.

One thing that's a little unexpected here is that the function passed in here (again, updateSquare from App) is a function that builds a function, with the parameters locked in by closure semantics. Closure semantics are definitely the way we would usually approach this, but by making this control deal with them rather than the caller. Rather than updateSquares needing to know that it's going to be called during render, and needing to create a new function, we'd typical do something like:

  return <button
    className="square"
    onClick={() => onClickCallback(id, value)}
  >
    {value}
  </button>

where updateSquares no longer needs to start with return () => {. It would just include the inner logic. It's the job of the child control to defer calling the passed in handler until the button is actually clicked (by wrapping it with an anonymous function) rather than the parent needing to know that's how the callback is going to be used.

As it is, the wave 2 Square test does pass, but it passes even if we comment out the simulated click. The passed handler gets called as part of the rendering of the square component, as opposed to being called when the button is clicked.

[anselrognlie]

for/of loops can use const loop variables. We should always prefer const over let unless our logic requires us to use let. Even in that case, we should think about whether we can restructure our logic to use const.

[anselrognlie]

If you find it annoying that you need to repeat the names and values of these attributes consider something like the following:

<Square key={square.id} {...{...square, onClickCallback}} />

which creates a new object with the spread contents of square, and value in onClickCallback being stored under the key onClickCallback. This new object is itself spread within the attribute list area of the Square component, which turn the key-value pairs into attributes with values on the component!

[anselrognlie]

Notice that there's a lot of console noise created by line 29:

console.log(squareList);

That line was mostly there to give you some visual indication whether the array was being flattened properly, and could be removed.

[anselrognlie]

This code can be made functional by retrieving the the flattened squares, and mapping them down to their value as follows. (Add the first line, and change the variable name used for the rest of the checks). Without a similar change, winner detection doesn't work.

    const flatSquares = squares.flat().map(s => s.value);
    for (let i = 0; i < possibleLines.length; i++) {
      const [a, b, c] = possibleLines[i];
      if (flatSquares[a] && flatSquares[a] === flatSquares[b] && flatSquares[a] === flatSquares[c]) {
        return flatSquares[a];
      }
    }

[anselrognlie]

As mentioned above, we'd like to have a piece of state to hold the winner, so the click handler has access to a value to use to tell whether the game is over (and then ignore further clicks). If we add the following piece of state up with the rest of the state values

  const [winner, setWinner] = useState('');

then we can add code such as the following to update that state. Notice that we need to be a little careful to add checks to prevent us from going into an infinite render loop!

  const newWinner = checkForWinner();
  if (! winner && newWinner) {
    setWinner(newWinner);
  }

[anselrognlie]

Once a winner has been found, we don't want to display the current player info anymore. So which of these we want to display is really governed by whether we've found a winner yet. This is another argument in favor of adding some winner state (there are ways to avoid this, but using the state is probably the most direct). Once we have that winner state (as shown above), we could rewrite these lines to use that state as something like:

        <h2>{ winner ? `Winner is ${winner}` : `Current Player ${player}` }</h2>

Notice that I removed the : from the winner display. This is to make the tests happy. The winner check doesn't expect a : in the display string. Of course, if you want to include a :, the test could be updated to match that.

[anselrognlie]

The winner tests (as well as any other test checking for x/o) would all need to be updated to look for 🍿/🍭.

[anselrognlie]

It doesn't look like you implemented the optional reset functionality, so you could leave these skipped.