Just finished game of life!

Code Feedback
1

Hey guys! I just finished Conway’s game of life. I’m really liking it.
My codepen here.

1 Like
2

Cool, good job, I like the pulsing effect.

Taking a look at the code…

const App = () => {
  const [board, setBoard] = useState(makeBoard(-1));
  return (
  <div>
    <h1 id="Title">Conway's Game of Life</h1>
    <div className="flex">
      <Board board={[board, setBoard]}/>
      <Menu board={[board, setBoard]}/>
    </div>
  </div>);
}

This just looks odd to me. You destucture the board and setBoard from the array that useState returns and then just combine them back into an array when you pass it to the children.

I have a couple of issues with this:

  1. It’s wasteful.
  2. You have a variable here called “board” that means one thing, and then in the child it will mean something entirely different. That is the sort of thing that leads to big confusion later. Is “board” the board or is it an array that has the board?
  3. Creating array literals like that could (I think) cause unnecessary rerenders. Everytime this component rerenders, the child will receive a new reference to a newly created array and will think it is getting new props and rerender.
  const [boardn, setBoardn] = board;

What I was talking about with name confusion? It leads to things like this.

  return (<div className="wrapper">
    <div className="board">
    {
    boardn
      .map(

This is very poorly formatted. I know it’s hard when you’re starting out but try to develop some good habits early, learn to format as you code - it saves a lot of time in the long run. When you get to coding in “the real world” you’ll be on a local editor and you can install a linter to tell you when your formatting is off. For now, in codepen, you can select all your code and press the dropdown in the upper right corner to select code formatting.

This is a little confusing:

  return (<div className="wrapper">
    <div className="board">
    {
    boardn
      .map(
        (i, index1) => i.map(
            (j, index2) => <div
                id={[index1, index2]}
                style={color(j)}
                onMouseDown={handleDown}
                onMouseEnter={handleEnter}
                style={{animationName: j === 1 ? "deadOrAlive":"none"}}
                onMouseUp={handleUp}
                key={index1+""+index2}
                className="body"
            ></div>
        )
      )
    }
    </div>
  </div>)

You have a map returning an array of JSX that is it itself part of a callback in another map that is returning JSX and is itself in JSX. That’s two many Cuils for my taste.

I would have at least put that inner callback into a its own, well-named function. Maybe I’d do it to both.

And this:

                id={[index1, index2]}

Most people when they think of an id, they assume it is a string or a number, not an array. I think it gets converted to string, like “1,2”, but I would do it myself.

This:

                style={color(j)}

Does color return a color? Does it “color” something? I would call this getStyle or something like that.

This:

      board[i].push(num === -1 ? (Math.random() < 0.5 ? 0 : 1) : num);

What about:

      board[i].push(num === -1 ? Math.round(Math.random()) : num);

I don’t know. I’m not sure which I like better. It just popped into my head.

Things like this:

        if(j === 1)
          if(!findNeightbours(j, index1, index2, list))
            return 0;

Why not:

        if(j === 1 && !findNeightbours(j, index1, index2, list))
          return 0;

I think that section could be simplified some more - but I don’t want to be too much of a a pest.

Still, all in all it looks good. Keep in mind that I’m a very tough code reviewer, so don’t take it to hard. Just try to understand what I’m saying and figure out what works best for you.

I was going to say, "have fun on … [the next project], but I guess it isn’t one of the FCC projects anymore. Here’s my old one if you’re curious. I’m sure there would be a lot I would pick on if I reviewed this now - there’s nothing more painful that looking at old code. The “Presets” thing doesn’t seem to be working, but I’m too lazy to fix it.

Sadly, Conway was died from Covid last year.

1 Like
3

What am I reading man? You looked at my code and gave me a lot of feedback and made me a better developer. To me, you are a brother.

this blew my brain by the way:

Thank you. I’m very happy and sad, I had no idea about Conway.

4

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.