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

Emulating Conway's Game of Life using JavaScript

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

Problem

I am currently reading Eloquent JavaScript and am trying to solve the exercise problems. One of the exercises in Chapter 18 asks to emulate Conway's Game of Life using checkboxes in JavaScript.

Before looking at the official solution, I like to solve the exercise on my own and then compare it with the author's solution. This time, though, I am a little confused.

My solution:

```

Conway's game of life

h1 {
margin-top : 50px;
font-family : Cardo;
}
#world{
width : 800px;
height : 400px;
margin : 0 auto;
margin-top :100px;
padding : 0px;
}

input[type="checkbox"] {
margin : 0px;
margin-top : 5px;
margin-left : 5px;
}




Conway's game of life




'use strict';
// World is represented by a div and cells in the world are represented by checkboxes

// As defined by the browser default styles at zoom level of 100%
var WIDTH_CELL = 17;
var HEIGHT_CELL = 17;

// Div that holds the world of the cells
var world = document.querySelector("#world");
var HEIGHT_WORLD = world.getBoundingClientRect().height;
var WIDTH_WORLD = world.getBoundingClientRect().width;

var NUM_ROWS = Math.floor(HEIGHT_WORLD / HEIGHT_CELL);
var NUM_COLS = Math.floor(WIDTH_WORLD / WIDTH_CELL);
var NUM_CELLS = NUM_ROWS * NUM_COLS;

// Holds every cell element that exists in the DOM
var cells = [];
// Fill the world with cells
for (var i = 0 ; i 3)
cellsThatDie.push(currentCell);
else if(numLiveNeighbors === 2 || numLiveNeighbors === 3)
cellsThatLive.push(currentCell);
}else{
if(numLiveNeighbors === 3)
cellsThatLive.push(currentCell);

Solution

No, your implementation is not gratuitously complicated.

Most of what you wrote extra is going to pay off if you ever want to extend the app (to use other rules, other sorts of neighborhoods, cell types and so on). You also provided an extra Vector class in there which isn't per se part of Game of Life - you can pretend it comes as a separate math library. There are however bits and pieces that can be simplified.

The cellsThatLive/Die are not really necessary. Compare your incrementGeneration with the author's nextGeneration. The function should't take the old grid from the outer scope, but rather as a parameter - this allows you to use the function separately elsewhere, in another program or from a unit test. Also, I'd separate the data from the presentation - the function that computes the next generation should have nothing to do with html elements; it should only move data around.

This

setInterval(function(){
    incrementGeneration();
}, 150);


can be rewritten simply as setInterval(incrementGeneration, 150);; there's no need for the extra function wrap.

You have a bunch of for loops that can be simplified if you use forEach, map and the like (notice the author uses them). Old-school for loops contain a LOT of code/extra information: you declare a variable, you initialize it, you do this and that when you actually just want to iterate.

The getLiveNeighborsCount function creates an array neighbors but you only use it to retrieve its length - you can just count directly. Oh and It's bad practice to mutate objects passed as arguments (I'm talking about the addNeighborIfValid function (mutating most things should be considered bad practice IMO)).

neighborDirections can be a simple array since you're not doing anything with the keys.

Update

Regarding modifying (mutating) parameters, it's best avoided. It's easier to reason about functions if you know they just LOOK at the data they are presented with and do not TOUCH it - the function can of course return a value and you decide what to do with it.

This is a common pattern that I see a lot:

// some function that loads an image given an URL
// and also may take a bunch of options { format, resize and whatnot }
function loadImage(url, options) {
    // options is optional itself
    options = options || {};

    // options have defaults
    // and this is where the "crime" occurs
    options.format = options.format || 'RGB';
    options.resizeX = options.resizeX || 640;
    options.resizeY = options.resizeY || 480;

    // load the image and use options
    var image = new Image() ...
    image.src = url ...
    resize(options.resizeX, options.resizeY...
}


If I call this function loadImage('blaha.jpg', options) I'm now going to get my image, but I'm also going to get my original options object contaminated with stuff that I never asked for - these are unintended side effects.

Also, take a look at a related question: https://softwareengineering.stackexchange.com/questions/245767/is-it-an-antipattern-modifying-an-incoming-parameter

Code Snippets

setInterval(function(){
    incrementGeneration();
}, 150);
// some function that loads an image given an URL
// and also may take a bunch of options { format, resize and whatnot }
function loadImage(url, options) {
    // options is optional itself
    options = options || {};

    // options have defaults
    // and this is where the "crime" occurs
    options.format = options.format || 'RGB';
    options.resizeX = options.resizeX || 640;
    options.resizeY = options.resizeY || 480;

    // load the image and use options
    var image = new Image() ...
    image.src = url ...
    resize(options.resizeX, options.resizeY...
}

Context

StackExchange Code Review Q#87330, answer score: 2

Revisions (0)

No revisions yet.