patternjavascriptMinor
Minesweeper game for Node.js
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
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
The function
Other suggestions
-
excess ternary operators - e.g.
To:
-
excess logic -
To:
-
excess
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 bothforloops.
-
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 excesselsewith pointlessreturn
- the arrow function in callback to
forEachinMineBoard:: winGame()doesn’t needelsesince previous blocks havereturnstatements
- Excess
breakstatements inMineBoard::determineNumberOfMines()because each case has areturnstatement.
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.