9
\$\begingroup\$

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 (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 game\n` +
        `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!

\$\endgroup\$
  • 1
    \$\begingroup\$ I wish someone would review this - I think it's a great candidate for review on this site. +1'd \$\endgroup\$ – maxwell Nov 9 '17 at 20:51
3
\$\begingroup\$

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.
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.