Minesweeper in React -- design for state management/data flow

Question

I've pasted here a basic implementation I've done of a Minesweeper game. This is not the full functionality, only the populating/revealing of squares. The idea is that the user inputs rows,cols,list-of-bombs as a string, and the game generates a board of the input dimensions, with bombs at the input indices.

I'm specifically wondering about maintaining the state of all these cells. I originally stored the board and visibility list as 2d arrays, but that didn't seem right.

I would really like to learn if my strategy for state management was good. I had a similar question on a take home for a trading desk role at a HFT firm, and I failed, so I'm trying to see if I can learn from my experience.

I'm also wondering why my <Cell /> components keep rerendering each time a cell is toggled.

// LIB
const generateMap = <vals = number | boolean,>(
  rows: number,
  cols: number,
  defVal: vals
) => range(rows * cols).reduce((acc, curr) => ({ ...acc, [curr]: defVal }), {});

const range = (num: number) => {
  const res = [];
  let count = 0;
  while (count < num) {
    res.push(count++);
  }
  return res;
};

const addBombs = (values: Record<string, number>, bombs: string[]) => ({
  ...values,
  ...bombs.reduce((acc, curr) => ({ ...acc, [curr]: -1 }), {}),
});

const addValues = (
  values: Record<string, number>,
  row: number,
  col: number
) => {
  const getCount = (key: number) => (values[key] == -1 ? 1 : 0);

  for (const key in values) {
    if (values[key] == -1) {
      continue;
    }
    const keyNum = Number(key);
    const rowCount = Math.floor(keyNum / row);
    const colCount = keyNum % col;

    const left = colCount > 0 ? getCount(keyNum - 1) : 0;
    const right = colCount < col - 1 ? getCount(keyNum + 1) : 0;
    const up = rowCount > 0 ? getCount(keyNum - col) : 0;
    const down = rowCount < row - 1 ? getCount(keyNum + col) : 0;
    const topLeft =
      colCount > 0 && rowCount > 0 ? getCount(keyNum - col - 1) : 0;
    const topRight =
      colCount < col - 1 && rowCount > 0 ? getCount(keyNum - col + 1) : 0;
    const downLeft =
      colCount > 0 && rowCount < row - 1 ? getCount(keyNum + col - 1) : 0;
    const downRight =
      colCount < col - 1 && rowCount < row - 1 ? getCount(keyNum + col + 1) : 0;

    values[key] =
      left + right + up + down + topLeft + topRight + downLeft + downRight;
  }
  return values;
};



// COMPONENTS
const Input = ({ handlePlay }: { handlePlay: (val: string) => void }) => {
  const [val, setVal] = useState("");

  return (
    <div
      style={{
        display: "flex",
        alignItems: "center",
        justifyContent: "center",
        marginBottom: 20,
      }}
    >
      <input type="text" value={val} onChange={(e) => setVal(e.target.value)} />
      <button onClick={() => handlePlay(val)}>Play</button>
    </div>
  );
};

const Cell = React.memo(
  ({
    visible,
    value,
    handleClick,
  }: {
    visible: boolean;
    value: number;
    handleClick: () => void;
  }) => {
    console.log('cell rendering')
    return (
      <div
        onClick={() => handleClick()}
        style={{
          padding: 10,
          display: "flex",
          justifyContent: "center",
          alignItems: "center",
          width: "30px",
          height: "30px",
          border: "1px solid grey",
          backgroundColor: visible ? "white" : "lightgrey",
        }}
      >
        {visible && (value > -1 ? value : "💣")}
      </div>
    );
  }
);

const Grid = ({
  seen,
  values,
  rows,
  cols,
  handleCellClick,
}: {
  seen: Record<string, boolean>;
  values: Record<string, number>;
  rows: number;
  cols: number;
  handleCellClick: (key: string) => void;
}) => {
  const key = (row: number, col: number) => (row * cols + col).toString();

  return (
    <div
      style={{
        display: "flex",
        flexDirection: "column",
        alignItems: "center",
        justifyContent: "center",
      }}
    >
      {range(rows).map((r) => (
        <div
          style={{
            display: "flex",
            alignItems: "center",
            justifyContent: "center",
          }}
        >
          {range(cols).map((c) => (
            <Cell
              key={key(r, c)}
              handleClick={() => handleCellClick(key(r, c))}
              value={values[key(r, c)]}
              visible={seen[key(r, c)]}
            />
          ))}
        </div>
      ))}
    </div>
  );
};

function App() {
  const [values, setValues] = useState<Record<string, number>>({});
  const [seen, setSeen] = useState<Record<string, boolean>>({});
  const [[rows, cols], setRowCol] = useState<number[]>([]);

  const handlePlay = (val: string) => {
    const [row, col, ...bombs] = val.split(",");

    const valuesMap = generateMap(Number(row), Number(col), 0);
    const seenMap = generateMap(Number(row), Number(col), false);

    setValues(addValues(addBombs(valuesMap, bombs), Number(row), Number(col)));
    setSeen(seenMap);
    setRowCol([Number(row), Number(col)]);
  };

  const handleCellClick = useCallback(
    (key: string) => {
      setSeen({
        ...seen,
        [key]: true,
      });
    },
    [seen]
  );

  return (
    <div>
      <Input handlePlay={handlePlay} />
      <Grid
        rows={rows}
        cols={cols}
        seen={seen}
        values={values}
        handleCellClick={handleCellClick}
      />
    </div>
  );
}

export default App;

Answer 1

I'd say it shouldn't ultimately matter how you store it underneath. There's some advantage for keeping everything in a 1D array for React in making new references quickly.

The important thing is to abstract the manipulation of state so it doesn't matter and put a boundary between game logic and state. I'd keep the game logic around the idea row/col pairs a long as I absolutely could and only start dealing with the implementation when I had to. You're doing this earlier than necessary in addValues when you could theoretically push it off to getCount with a change of signature.

I think you're on the right track with the addValues interface (pass in the array, give it a column and number), but the key as a string is definitely awkward. You're going to have unique values from row * cols + col, so I'd just use a simple array and index it that way.

I'd really consider making a new Values class which gets your generateMap as the constructor, remembers the sizes, and then holds the values as an array. It gets instantiated each render with the values from the state. The values later can be retrieved via a getter which returns a copy you can then feed into your setValues state after manipulation. I'd expect introducing a class doesn't hurt performance, but do keep it in mind.

In any scenario, you can have a simpler addValues which double loops over rows and cols (or this.rows, this.cols if using a class). getCount can take the row/col pair and calculate the index via row * this.ncols + col rather than the string -> number -> floor -> juggle keyNum math. I think this should make the calculation of the values around the cell more readable.

Also, you can replace your range function with something like

[...Array(num).keys()]

Answer 2

The code seems well formatted and the variables are generally well chosen.

---

The main issue is that somebody needs to be pretty deep into function programming to understand what is going on at all.

Constructs such as:

const addBombs = (values: Record<string, number>, bombs: string[]) => ({
  ...values,
  ...bombs.reduce((acc, curr) => ({ ...acc, [curr]: -1 }), {}),
});

are hard to read (in my opinion). Considering that it simply puts the bombs in place I'm pretty sure that readability would be greatly enhanced if simple procedural code was being used.

---

The fact that it uses stringified position in an one-dimensional array rather to use a position muddles the water, but I guess that comes with the restrictions build into JavaScript & TypeScript.

One way around it would be to create an object that can be serialized and deserialized. That would increase the code size and it would still require explicit calls to the serialization method, but if they would serialize to "(<x>, <y>)" then this would help with debugging.

---

There are too many literals in the code. For instance, the fact that apparently -1 represents a bomb must be retrieved from the code.

---

The part that calculates the number of bombs contains a lot of inline calculations that are hard to understand. I'd use a different abstraction using positions and directions instead.

For instance, you could define constants for directions such as Left, Down, Right and Up with values [-1, 0], [0, -1], [1, 0] and [0, 1]. That way the code would become much more clear.

---

Keys are calculated as strings. This is OK, as TypeScript does always treat the keys within a Record as a symbol or key. Still, it might be a better idea to configure a record as <number, number> to more clearly convey what it is for. Or use a string that conveys a position more clearly as mentioned earlier.

---

There are many separate calls to getCount and key(r, c) in the code. In principle it is good that these are separate functions.

That said, when I'm writing code I do try to make sure that I call methods as few times as possible. That's certainly possible for getCount if e.g. you're iterating through neighbors of a cell.

For key this may be a bit harder to do because you're already in a lambda, but it is possible:

// Capture the output of the key function in a variable
const keyValue = key(r, c);
return (
  <Cell
    key={keyValue}
    handleClick={() => handleCellClick(keyValue)}
    value={values[keyValue]}
    visible={seen[keyValue]}
  />
);