patternjavascriptMinor
One of iteration Conway's Game of Life
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
This or any other readability/optimization advice would be greatly appreciated.
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
this way the first definition of
-
Don't use
-
If I am in control of the design, something I often do is add a boundary around my matrix. Something like:
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:
btw, There is a nice feature in most environments. You can use:
-
Nest the other functions inside of
oneIterationConwayfunction 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.