HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

One of iteration Conway's Game of Life

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
conwayiterationonegamelife

Problem

I would really like to get some feedback on how you would improve my code's style, readability, and efficiency. I failed an interview recently and was unable to get any feedback so I thought I would try asking here. I have cleaned up the code a bit already based on advice from friends but would like to have more critical eyes take a look.

My task was to write a function that takes a matrix of 1's and 0's and returns a resulting matrix after one iteration of Conway's Game of Life.

Note: Since the goal was to only return 1 iteration, I was told to not create cell objects and hold references to neighbors. There is not a need to set this up because we only want to run 1 iteration and doing so would waste space.

I would like advice on what you would change in terms of the function organization and how I decided to break down the problem. Specifically, it feels a little awkward how I have to pass the matrix all the way down to the isInBounds function. Is there a way to avoid this?

This or any other readability/optimization advice would be greatly appreciated.

function oneIterationConway(matrix) {
  const height = matrix.length;
  const width = matrix[0].length;
  const resultMatrix = [];

  for (let row = 0; row  1 && neighborCount = 0 && col >= 0 && row < height && col < width;
}

const testMatrix = [
  [1, 0, 0, 0, 0],
  [0, 1, 1, 0, 0],
  [0, 1, 0, 0, 0]
];

const result = oneIterationConway(testMatrix);

for (let row of result) {
  console.log(row);
}

Solution

There are a couple of ways of solving this:

-
Nest the other functions inside of oneIterationConway

function oneIterationConway(matrix) {
  const height = matrix.length;
  const width = matrix[0].length;
  function isInBounds(row, col, matrix) {
    return row >= 0 && col >= 0 && row < height && col < width;
  }


this way the first definition of height is in scope (available) inside isInBounds. Hiding your 'private' functions is a good idea anyway.

-
Don't use isInBounds at all, take advantage of the fact that items out of bounds are undefined.

function countNeighbors(row, col, matrix) {
  let neighbors = -matrix[row, col];

  for (let i = -1; i <= 1; i++) {
    for (let j = -1; j <= 1; j++) {
      let value = matrix[row+i, col+j];
      if (value) neighbors++;
    }
  }
  return neighbors;
}


-
If I am in control of the design, something I often do is add a boundary around my matrix. Something like:

const testMatrix = [
  [0, 0, 0, 0, 0, 0, 0],
  [0, 1, 0, 0, 0, 0, 0],
  [0, 0, 1, 1, 0, 0, 0],
  [0, 0, 1, 0, 0, 0, 0],
  [0, 0, 0, 0, 0, 0, 0],
];


My loops would then be from 1 to size-2

The only other obvious thing I would be would be to factor out part of the main loop:

for (let row = 0; row  1 && neighborCount < 4 ? 1 : 0;
  } else {
    // current cell is dead
    return neighborCount === 3 ? 1 : 0;
  }
}


btw, There is a nice feature in most environments. You can use: console.table(results)

Code Snippets

function oneIterationConway(matrix) {
  const height = matrix.length;
  const width = matrix[0].length;
  function isInBounds(row, col, matrix) {
    return row >= 0 && col >= 0 && row < height && col < width;
  }
function countNeighbors(row, col, matrix) {
  let neighbors = -matrix[row, col];

  for (let i = -1; i <= 1; i++) {
    for (let j = -1; j <= 1; j++) {
      let value = matrix[row+i, col+j];
      if (value) neighbors++;
    }
  }
  return neighbors;
}
const testMatrix = [
  [0, 0, 0, 0, 0, 0, 0],
  [0, 1, 0, 0, 0, 0, 0],
  [0, 0, 1, 1, 0, 0, 0],
  [0, 0, 1, 0, 0, 0, 0],
  [0, 0, 0, 0, 0, 0, 0],
];
for (let row = 0; row < height; row++) {
  resultMatrix[row] = [];
  for (let col = 0; col < width; col++) {
    resultMatrix[row][col] = isAlive(row, col);
  }
} 

function isAlive(row, col) {
  const neighborCount = countNeighbors(row, col);
  if (matrix[row][col]) {
    // current cell is alive
    return neighborCount > 1 && neighborCount < 4 ? 1 : 0;
  } else {
    // current cell is dead
    return neighborCount === 3 ? 1 : 0;
  }
}

Context

StackExchange Code Review Q#158586, answer score: 2

Revisions (0)

No revisions yet.