Game of Life component in React

Question

I have been learning webdevelopment at Freecodcamp and have therefore been writing a lot of javascript by myself. They have a code-review chatroom, but the people in that chatroom seem to review the final product and not the code itself.

So, I am searching for some constructive criticism about the structure and style of my code.

The code shown below is a class from a Game of Life game as part of the Freecodecamp course and does work. It is written in Javascript and uses React. There is more code for this project, but I don't think it is necessary to show all of the code, for you to critique my code structure and style. If it the rest of the code is required I can of course post it as well.

class GameOfLife extends Component {
 constructor(props) {
 super(props)
 this.toggleGameState = this.toggleGameState.bind(this);
 this.stepGame = this.stepGame.bind(this);
 this.squareClicked = this.squareClicked.bind(this);
 this.clearGrid = this.clearGrid.bind(this);
 this.generateRandomGrid = this.generateRandomGrid.bind(this);
 this.toggleSpeed = this.toggleSpeed.bind(this);
 this.state = {
 cellRows: generateRandomCellDistribution({height: 50, width: 50}), 
 size: {height:50, width:50}, 
 gameState: 'playing', speed: 1,
 generation: 0
 };
 }
 componentDidMount() {
 this.setState({gamteState: 'playing', interval: setInterval(this.stepGame, 200/this.state.speed)});
 }
 toggleGameState() {
 var newState = this.state.gameState === 'playing' ? 'paused' : 'playing';
 if(newState === 'playing') {
 this.setState({gameState: newState, interval: setInterval(this.stepGame, 200/this.state.speed)});
 } else {
 this.setState({'gameState': newState});
 clearInterval(this.state.interval);
 }
 }
 squareClicked(row, column) {
 var cellRows = this.state.cellRows.slice();
 var newState = cellRows[row][column].state === 'alive' ? 'dead' : 'alive';
 cellRows[row][column] = {state: newState};
 this.setState({cellRows: cellRows});
 }
 generateRandomGrid() {
 this.setState({cellRows: generateRandomCellDistribution(this.state.size), generation: 0})
 }
 clearGrid() {
 this.setState({cellRows: getBlankCellRows(this.state.size), generation: 0});
 }
 toggleSpeed(){
 var newSpeed = this.state.speed < 4 ? this.state.speed+1 : 1;
 clearInterval(this.state.interval);
 this.setState({speed: newSpeed, interval: setInterval(this.stepGame, 200/newSpeed)});
 }
 stepGame() {
 var newCellRows = getBlankCellRows(this.state.size);
 var currentCellRows = this.state.cellRows.slice();
 var aliveNeighbours = getAliveNeighbours(currentCellRows);
 for(var i = 0; i < currentCellRows.length; i++) {
 for(var q = 0; q < currentCellRows[i].length; q++) {
 newCellRows[i][q].state = getNewState(currentCellRows[i][q].state, getAliveNeighbours(currentCellRows, i, q));
 }
 }
 this.setState({cellRows: newCellRows, generation: this.state.generation + 1});
 }
 render(){
 return(
 <div>
 <div className="ui-container"> 
 <button className="button-theme" onClick={this.toggleGameState}>{this.state.gameState === 'playing' ? 'Pause' : 'Start'}</button>
 <button className="button-theme" onClick={this.clearGrid}>Clear grid</button>
 <button className="button-theme" onClick={this.generateRandomGrid}>Random</button>
 <button className="button-theme" onClick={this.toggleSpeed}>Speed: {this.state.speed}x</button>
 <div className="generation-indicator">Generation: {this.state.generation}</div>
 </div>
 <Grid 
 gameState={this.state.gameState} 
 size={this.state.size} 
 cellRows={this.state.cellRows} 
 squareClicked={this.squareClicked}
 />
 </div>
 );
 }
}

Answer 1

// Good: Object properties are in a logical order 
// (alphabetical order isn't always best)
this.state = {
 cellRows: generateRandomCellDistribution({height: 50, width: 50}), 
 size: {height:50, width:50}, 
 gameState: 'playing', 
 speed: 1, // This property should be on its own line like the rest.
 generation: 0
};
// Style should be consistent (Source: Code Complete)
// Object properties on their own lines like above
// (even as parameters).
componentDidMount() {
 this.setState({
 gamteState: 'playing',
 // No magic numbers (Source: Code Complete)
 // Why 200/speed? What does that represent?
 // Assign a variable above like var tickSpeed = 200/this.state.speed
 // Something to explain why you're doing the division.
 // Even better would be to put that in another function
 // to be used in all places you do this conversion. 
 interval: setInterval(this.stepGame, 200/this.state.speed)
 });
}
// Function name should describe what the function does
// (rather than what triggers it) (Source: Code Complete)
squareClicked(row, column) --> handleClickInSquare(row, column) or updateStateForCell(row, column)
// Same here. Function doesn't just return a random grid, 
// it changes the grid to be a random grid (affects state).
generateRandomGrid() --> setGridToRandomGrid() or randomizeGrid()
// This one is pretty good, but toggle usually refers to 
// something with two options like Caps Lock (Source: Google define).
// This is tricky and you might think up an even better name that
// includes the facts that you're increasing the speed, there are 
// several options, and it wraps around back to the first option.
toggleSpeed() --> cycleSpeed(), increaseSpeedOrWrap(), or selectNextFasterSpeed()
// It's better to select clear iterators although it's not
// uncommon to see i, j, and k. Not sure what q is, though.
// How about i --> row, q --> cell. (Source: Code Complete)
// I've changed the code to demonstrate my suggestions.
// I also recommend using currentCellGrid or similar
// to make it clear that you have a 2D array.
for(var row = 0; row < currentCellGrid.length; row++) {
 // Here you could add a variable to clarify what currentCellGrid[row] is.
 // Use more variables to document what the code is doing.
 // (Source: Code Complete)
 var currentRowCells = currentCellGrid[row];
 for(var cell = 0; cell < currentRowCells.length; cell++) {
 newCellGrid[row][cell].state = getNewState(currentRowCells[cell].state, getAliveNeighbours(currentCellGrid, row, cell));
 }
}