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

Minesweeper game for Node.js

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

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.

minesweeper-nodejs on GitHub

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

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. in MineBoard::generateNewBoard() and MineBoard::generateNewViewBoard() could be simplified using Array.fill() - could eliminate one or both for 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 conditional

if (viewBoard[cursorRow][cursorColumn] === this.marker) {
  return true;
} else {
  return false;
}


To:

return (viewBoard[cursorRow][cursorColumn] === this.marker) ;


-
excess else statements

  • MineBoard:: checkForWinGame() has excess else with pointless return



  • the arrow function in callback to forEach in MineBoard:: winGame() doesn’t need else since previous blocks have return statements



  • Excess break statements in MineBoard::determineNumberOfMines() because each case has a return statement.

Code Snippets

let rowInBounds = row >= 0 && row < size ? true : false;
let rowInBounds = row >= 0 && row < size;
if (viewBoard[cursorRow][cursorColumn] === this.marker) {
  return true;
} else {
  return false;
}
return (viewBoard[cursorRow][cursorColumn] === this.marker) ;

Context

StackExchange Code Review Q#154616, answer score: 3

Revisions (0)

No revisions yet.