Problem
I recently graduated from a coding bootcamp, and while it was great for the most part – I feel like it was lacking in learning good coding technique. I recently built this Minesweeper game for node.js, and I would love some feedback! Here is the link to the GitHub repo. I really appreciate your time – the feedback will help me greatly in my job search.
Here is the code:
index.js
'use strict';
const { promptForBoardSize, promptForDifficultyLevel, promptUserForInputs } =
require('./src/helpers');
const { MineBoard } = require('./src/MineBoard');
const readline = require('readline');
const rl = readline.createInterface({
input: process.stdin,
output: process.stdout,
terminal: true
});
// Allow keypress events to be emitted through stdin
readline.emitKeypressEvents(process.stdin);
// Deactivate gameplay key bindings until game starts
let gamePlay = false;
// Declare functions for toggling gameplay key bindings
function toggleGamePlayOn() {
gamePlay = true;
}
function toggleGamePlayOff() {
gamePlay = false;
}
// Initialze kepress listeners
process.stdin.on('keypress', (str, key) => {
if (key.ctrl && key.name === 'c') {
process.exit();
}
if (key.name === 'n') {
toggleGamePlayOff();
promptUserForInputs(rl, startGame);
}
if (key.name === 'r' && board) {
startGame(board.getSize(), board.getDifficulty());
}
if (key.name === 'i' && board) {
board.printLongInstructions();
}
if (key.name === 'h' && board) { //hide long instructions
board.printViewBoardToConsole();
}
if (gamePlay) { // Only handle keypress if in gameplay
handleKeyPress(key);
}
});
// Set up functionality for gameplay
function handleKeyPress(key) {
switch (key.name) {
case 'up':
case 'down':
case 'left':
case 'right':
board.moveCursor(key.name);
break;
case 'space':
board.uncoverSpace();
break;
case 'm':
board.markSpace();
break;
}
if (board.isGameOver()) {
toggleGamePlayOff();
}
}
// Initialize game board
let board;
function startGame(size, difficulty) {
board = new MineBoard(size, difficulty);
// Switch input from readline to listen to key events
toggleGamePlayOn();
board.printViewBoardToConsole();
}
promptUserForInputs(rl, startGame);
src/helpers.js
function promptForBoardSize(rl, nextFun, startGameFun) {
rl.clearLine();
console.log("x1B[2J");
console.log('Welcome to Minesweeper for Node.js!');
rl.question(`How big of board would you like to play with? Please ` +
`input a whole number (5 through 25)n`, answer => {
if (answer >= 5 && answer <= 25) {
console.log(`Playing with board size ${answer}n`);
nextFun(rl, startGameFun.bind(null, answer));
} else {
console.log(`Invalid input, prompting again...n`);
promptForBoardSize(rl, nextFun, startGameFun);
}
});
}
function promptForDifficultyLevel(rl, startGameFun) {
rl.question(`What difficulty level would you like to play at? 1, 2 or 3?n`,
answer => {
if (answer === '1' || answer === '2' || answer === '3') {
console.log(`Playing at difficulty level ${answer}n`);
startGameFun(answer);
} else {
console.log(`Invalid input, prompting again....n`);
promptForDifficultyLevel(rl, startGameFun);
}
});
}
function promptUserForInputs(rl, startGameFun) {
promptForBoardSize(rl, promptForDifficultyLevel, startGameFun);
}
module.exports = {
promptForBoardSize,
promptForDifficultyLevel,
promptUserForInputs
};
src/MineBoard.js
'use strict';
/* Instantiate a WeakMap to hold private MineBoard class variables */
let _privateProps = new WeakMap();
class MineBoard {
constructor(size, difficulty) {
// Set the display values
this.cursor = String.fromCharCode(9670);
this.marker = String.fromCharCode(9873);
this.uncoveredSpace = ' ';
this.coveredSpace = String.fromCharCode(9634);
this.bomb = String.fromCharCode(9762);
// Store props in WeakMap to restrict access to class methods
let props = {};
_privateProps.set(this, props);
// This map allows for easy iteration of all squares surrounding each square
props.relationalIndicesMap = [
{row: -1, col: -1},
{row: -1, col: 0},
{row: -1, col: 1},
{row: 0, col: -1},
{row: 0, col: 1},
{row: 1, col: -1},
{row: 1, col: 0},
{row: 1, col: 1},
];
props.size = size || 20;
props.difficulty = difficulty || '1';
props.totalNumOfSquares = props.size ** 2;
props.numberOfMines = this.determineNumberOfMines();
// Generate new board requires number of mines to be determined first
props.mineBoard = this.generateNewBoard();
props.uncoveredCount = 0;
props.winningUncoveredCount = props.totalNumOfSquares - props.numberOfMines;
props.viewBoard = this.generateNewViewBoard();
props.cursorRow = 0;
props.cursorColumn = 0;
props.gameOver = false;
}
determineNumberOfMines() {
let { difficulty, totalNumOfSquares } = _privateProps.get(this);
switch(difficulty) {
case '1':
return Math.floor(totalNumOfSquares * 0.075);
break;
case '2':
return Math.floor(totalNumOfSquares * 0.1);
break;
case '3':
return Math.floor(totalNumOfSquares * 0.125);
}
};
generateNewBoard() {
let { size, numberOfMines } = _privateProps.get(this);
let board= [];
// Create board of zeros
for (let i = 0; i < size; i++) {
board[i] = [];
for (let j = 0; j < size; j++) {
board[i][j] = 0;
}
}
// Generate random mine placement
for (let i = 0; i < numberOfMines; i++) {
let row = Math.floor(Math.random() * size);
let column = Math.floor(Math.random() * size);
// If no mine at indices, place mine
if (board[row][column] === 0) {
board[row][column] = this.bomb;
} else {
i--;
}
}
//Generate proximity numbers
this.determineMineProximityNumbers(board);
return board;
}
determineMineProximityNumbers(board) {
let { relationalIndicesMap, size } = _privateProps.get(this);
for (let row = 0; row < size; row++) {
for (let column = 0; column < size; column++) {
// Skip over mines
if (board[row][column] === this.bomb) {
continue;
}
// Keep track of number of bombs adjacent to current indices
let numOfBombsInVicinity = 0;
// Iterate over 8 surround squares
relationalIndicesMap.forEach(relIndex => {
let rowToCheck = row + relIndex.row;
let colToCheck = column + relIndex.col;
if (!this.isInBounds(rowToCheck, colToCheck)) return;
if (board[rowToCheck][colToCheck] === this.bomb) {
numOfBombsInVicinity++;
}
}, this);
// Label square with result
board[row][column] = numOfBombsInVicinity;
}
}
}
generateNewViewBoard() {
let { size } = _privateProps.get(this);
let viewBoard = [];
for (let i = 0; i < size; i++) {
viewBoard[i] = [];
for (let j = 0; j < size; j++) {
viewBoard[i][j] = this.coveredSpace;
}
}
return viewBoard;
}
isInBounds(row, col) {
let { size } = _privateProps.get(this);
let rowInBounds = row >= 0 && row < size ? true : false;
let colInBounds = col >= 0 && col < size ? true : false;
return rowInBounds && colInBounds;
}
printViewBoardToConsole() {
let { cursorRow: row, cursorColumn: col, size, viewBoard } =
_privateProps.get(this);
// Clear the console before printing
this.clearConsole();
// Make copy of board for purpose of printing with cursor
let board = JSON.parse(JSON.stringify(viewBoard));
// Update the view at current cursor location
board[row][col] = this.cursor;
// Print updated viewboard with cursor location to console
board.forEach(row => {
console.log(row.join(' '));
});
// Only print short instructions each time.
this.printShortInstructions();
}
moveCursor(direction) {
let props = _privateProps.get(this);
let { cursorRow: row, cursorColumn: col } = _privateProps.get(this);
// Change cursor location based on direction
switch(direction) {
case 'up':
props.cursorRow = this.isInBounds(row - 1, col) ? row - 1 : row;
break;
case 'down':
props.cursorRow = this.isInBounds(row + 1, col) ? row + 1 : row;
break;
case 'left':
props.cursorColumn = this.isInBounds(row, col - 1) ? col - 1 : col;
break;
case 'right':
props.cursorColumn = this.isInBounds(row, col + 1) ? col + 1 : col;
}
// Reprint board to console
this.printViewBoardToConsole();
}
markSpace() {
let { viewBoard: board, cursorRow: row, cursorColumn: col } =
_privateProps.get(this);
// If the space is uncovered and unmarked, mark it
if (board[row][col] === this.coveredSpace) {
board[row][col] = this.marker;
} else if (board[row][col] === this.marker) { // If marked, unmark it
board[row][col] = this.coveredSpace;
}
}
isSpaceMarked() {
let { viewBoard, cursorRow, cursorColumn } = _privateProps.get(this);
if (viewBoard[cursorRow][cursorColumn] === this.marker) {
return true;
} else {
return false;
}
}
uncoverSpace() {
let props = _privateProps.get(this);
let { cursorRow: row, cursorColumn: col, mineBoard, viewBoard, size } =
_privateProps.get(this);
// Do not allow uncovering of marked spaces
if (this.isSpaceMarked()) {
return;
}
// Call game over if bomb detected
if (mineBoard[row][col] === this.bomb) {
this.gameOver();
return;
}
// If space is anything but a zero, uncover it
if (mineBoard[row][col] !== 0) {
viewBoard[row][col] = mineBoard[row][col];
props.uncoveredCount++;
this.printViewBoardToConsole();
this.checkForWinGame();
return;
}
// If space is zero, uncover adjacent spaces as well
if (mineBoard[row][col] === 0) {
uncoverAdjacentSpaces(row, col, this);
this.printViewBoardToConsole();
this.checkForWinGame();
return;
}
// Function to uncover adjacent spaces if space is not adjacent to any bombs
function uncoverAdjacentSpaces(row, col, thisArg) {
// Space is a zero (not adjacent to any bombs)
// Uncover, increase count
viewBoard[row][col] = thisArg.uncoveredSpace;
props.uncoveredCount++;
/*
Check Adjacent Squares. Uncover all non-bombs, and recurse on zeros
*/
props.relationalIndicesMap.forEach(relIndex => {
let rowToCheck = row + relIndex.row;
let colToCheck = col + relIndex.col;
if (!thisArg.isInBounds(rowToCheck, colToCheck)) return;
if (viewBoard[rowToCheck][colToCheck] !== thisArg.coveredSpace) return;
if (mineBoard[rowToCheck][colToCheck] === thisArg.bomb) return;
if (mineBoard[rowToCheck][colToCheck] > 0) {
viewBoard[rowToCheck][colToCheck] = mineBoard[rowToCheck][colToCheck];
props.uncoveredCount++;
} else { // Space equals zero, so recurse
uncoverAdjacentSpaces(rowToCheck, colToCheck, thisArg);
}
}, this);
}
}
checkForWinGame() {
let { uncoveredCount, winningUncoveredCount } = _privateProps.get(this);
if (uncoveredCount >= winningUncoveredCount) {
this.winGame();
return;
} else {
return;
}
}
gameOver() {
let { mineBoard } = _privateProps.get(this);
this.clearConsole();
// Print modified board to console, showing bombs.
mineBoard.forEach(row => {
let updatedRow = row.map(value => {
return value === 0 ? ' ' : value;
});
console.log(updatedRow.join(' '));
});
console.log('GAME OVER');
this.printShortInstructions();
// Update Game Over Boolean
_privateProps.get(this).gameOver = true;
}
winGame() {
let { mineBoard } = _privateProps.get(this);
this.clearConsole();
// Print modified board to console, showing flags.
mineBoard.forEach(row => {
let updatedRow = row.map(value => {
if (value === 0) {
return ' ';
} else if (value === this.bomb) {
return this.marker;
} else {
return value;
}
}, this);
console.log(updatedRow.join(' '));
}, this);
console.log('YOU WIN!!!');
console.log(_privateProps.get(this).uncoveredCount)
this.printShortInstructions();
// Update Game Over Boolean
_privateProps.get(this).gameOver = true;
}
isGameOver() {
return _privateProps.get(this).gameOver;
}
printShortInstructions() {
console.log(
`Welcome to Minesweeper for Node.js! Press 'i' to to see instructions.n`
);
}
printLongInstructions() {
console.log(
`Welcome to Minesweeper for Node.js!n` +
`Use the arrow keys to move around.n` +
`Use the 'm' key to flag a box and the 'space' key to uncover one.n` +
`Press 'ctr + c' to quit,'r' to restart, and 'n' for a new gamen` +
`Press 'h' to hide these instructions.`
);
}
clearConsole() {
console.log("x1B[2J");
}
getSize() {
return _privateProps.get(this).size;
}
getDifficulty() {
return _privateProps.get(this).difficulty;
}
}
module.exports = {
MineBoard,
};
I’m looking for feedback regarding coding style, structure, readability, git etiquette and anything else. I’m looking forward to your feedback!
Solution
Bearing in mind that this code was posted ~2.5 years ago, I have some feedback. Perhaps you have learned a lot since then and the information below might not be new but it might help others as well.
General Feedabck
Overall the code looks okay. Readability is pretty good, though the path through the helper functions is a little tricky to comprehend at first glance. I like the GUI-like experience when moving around with arrow keys.
I haven’t really used a WeakMap before – that is an interesting application of it. If you really wanted to make variables private then you could employ the Revealing module pattern to only expose the publicly available items and not reveal the private data.
I would suggest using const
more – especially for any variable that doesn’t get re-assigned – e.g. in MineBoard::determineNumberOfMines()
the variables difficulty
and totalNumOfSquares
, in MineBoard::generateNewBoard()
the variables size
and numberOfMines
, etc..
The function toggleGamePlayOn
in index.js appears to only be used in one place, which makes me question the benefit of declaring it. Given that board
is used board
is used within functions yet declared outside those functions why not do the same with gamePlay
?
Other suggestions
- use
Array.fill()
– e.g. inMineBoard::generateNewBoard()
andMineBoard::generateNewViewBoard()
could be simplified usingArray.fill()
– could eliminate one or bothfor
loops. -
excess ternary operators – e.g.
MineBoard::isInBounds()
could be simplified to not use ternaries – just return the conditions. For example:let rowInBounds = row >= 0 && row < size ? true : false;
To:
let rowInBounds = row >= 0 && row < size;
-
excess logic –
MineBoard:: isSpaceMarked()
could be simplified to return conditionalif (viewBoard[cursorRow][cursorColumn] === this.marker) { return true; } else { return false; }
To:
return (viewBoard[cursorRow][cursorColumn] === this.marker) ;
-
excess
else
statementsMineBoard:: checkForWinGame()
has excesselse
with pointlessreturn
- the arrow function in callback to
forEach
inMineBoard:: winGame()
doesn’t needelse
since previous blocks havereturn
statements
- Excess
break
statements inMineBoard::determineNumberOfMines()
because each case has areturn
statement.