Minesweeper JavaScript Prototype

Question

I am looking for some review of a Minesweeper game that I made as my first JavaScript program. What I have here is simply the prototype for the minesweeper game while I handle the actual grid in a separate section, and I could care less about that. I'm just looking for a few pointers in the right direction in case I am using any bad practices.

As for the game itself, I store two 2D arrays. board is a 2D array I use to keep track of what values the hidden tiles have, 0-9 being the number of adjacent mines and 9 being a mine itself. visibleBoard is the tiles that the player can see, and it uses the same values as board except an additional value of 10 being a tile that is still hidden.

 "use strict";

//Javascript Prototype for a Minesweeper Game Object
function MinesweeperGame(rows = 8, cols = 8, mines = 10) {
    //Initialize Game State Variables
    this.rows = rows;
    this.cols = cols;
    this.mines = mines;
    this.gameover = false;
    this.score = 0;
    this.board = null;
    this.visibleBoard = null;

    this.initBoard = function() {
        this.board = [];
        this.visibleBoard = [];
        this.gameover = false;

        //If the tile is inside the boundaries of the board and is not a bomb, increment by 1
        function increment(r, c, board) {
            if(typeof board[r] !== "undefined" && typeof board[r][c] !== "undefined" && board[r][c] != 9) {
                board[r][c]++;
            }
        }

        //initializes the board with 0s and the visibleBoard with 10s
        for(var i = 0; i < this.rows; i++) {
            var arr1 = [];
            var arr2 = [];
            for(var j = 0; j < this.cols; j++) {
                arr1.push(0);
                arr2.push(10);
            }
            this.board.push(arr1);
            this.visibleBoard.push(arr2);
        }

        //Retrieve all squares in the board
        var squares = []
        for(i = 0; i < this.rows * this.cols; i++) {
            squares.push(i);
        }

        //Place bombs in a few squares
        for(i = 0; i < this.mines; i++) {
            var sq = squares.splice(Math.floor(Math.random() * squares.length), 1)[0];
            var r = Math.floor(sq / this.cols);
            var c = sq % this.cols;
            this.board[r][c] = 9;
        }

        //Increment all tiles adjacent to bombs
        for(var r = 0; r < this.rows; r++) {
            for(var c = 0; c < this.cols; c++) {
                //If a board sqaure is a bomb
                if(this.board[r][c] == 9){
                    //Add 1 to all non-bomb adjacent tiles
                    increment(r - 1, c - 1, this.board);
                    increment(r - 1, c, this.board);
                    increment(r - 1, c + 1, this.board);
                    increment(r, c - 1, this.board);
                    increment(r, c + 1, this.board);
                    increment(r + 1, c - 1, this.board);
                    increment(r + 1, c, this.board);
                    increment(r + 1, c + 1, this.board);
                }
            }
        }
    },

    this.makeMove = function(row, col) {
        if(this.gameover) {
            throw 'Game Over';
        }

        //If you choose an empty space, it flood fills out the rest of the empty squares
        function flood(r, c, board, visibleBoard) {
            if(typeof visibleBoard[r] !== "undefined" && typeof visibleBoard[r][c] !== "undefined" && visibleBoard[r][c] == 10) {
                if(board[r][c] == 0) {
                    visibleBoard[r][c] = 0;
                    flood(r - 1, c - 1, board, visibleBoard);
                    flood(r - 1, c, board, visibleBoard);
                    flood(r - 1, c + 1, board, visibleBoard);
                    flood(r, c + 1, board, visibleBoard);
                    flood(r, c - 1, board, visibleBoard);
                    flood(r + 1, c - 1, board, visibleBoard);
                    flood(r + 1, c, board, visibleBoard);
                    flood(r + 1, c + 1, board, visibleBoard);
                } else {
                    visibleBoard[r][c] = board[r][c]
                }
            }
        }

        //Will update the gameover variable if the user wins the game
        function checkGameOver(visibleBoard) {
            var count = 0;
            for(var r = 0; r < rows; r++) {
                for(var c = 0; c < cols; c++) {
                    if(visibleBoard[r][c] == 10) {
                        count++;
                    }
                }
            }
            return count == mines;
        }

        //Updates the visible board
        if(this.visibleBoard[row][col] == 10) {
            var val = this.board[row][col];
            if(val == 9) {
                this.gameover = true;
                this.visibleBoard[row][col] = 9;
                return 10;
            } else if(val > 0 && val < 9) {
                this.visibleBoard[row][col] = val;
                this.gameover = checkGameOver(this.visibleBoard);
                return val;
            } else if(val == 0) {
                flood(row, col, this.board, this.visibleBoard);
                this.gameover = checkGameOver(this.visibleBoard);
                return 0;
            }
        } else {
            throw 'Invalid Tile';
        }
    }
}

Primary Concerns:

1. Nested functions (e.g. increment, flood, and checkGameOver)
2. Appropriate use of variable scopes (I feel like I use this too often)
3. Efficient implementation of algorithms
4. Should I break some functionality down more or less?
5. Is there a better way to place down mines?

Answer 1

To address your concerns first:

Nested functions (e.g. increment, flood, and checkGameOver)

Nested functions are a great feature of JavaScript, and I'd say you're using them pretty well. However, there's certainly some repetition in the functions that have to find neighbors of a square/tile/cell. I'd consider make a neighborsOf(row, col) function that returns an array, and call that whenever you need the neighbors of a certain location. Similarly, I'd consider adding a forEach-like method that takes a callback and passes it each square on the board, so you only have to do the doubled for-loop in one place.

Appropriate use of variable scopes (I feel like I use this too often)

I wouldn't say you're using this too often. If you're accessing an instance variable from that instance, you use this. That's just the way it is.

However, you should use prototyping when defining the methods on MinesweeperGame. Right now you're adding methods to the instance, rather than "inheriting" them from a prototype. Not wrong, necessarily, but prototyping seems a better fit here. I.e. instead of this:

function MinesweeperGame {
  this.initBoard = function () { ... }
}

do this:

function MinesweeperGame {

}

MinesweeperGame.prototype = {
  initBoard: function () { ... }
};

(Incidentally, your comment for MinesweeperGame is incorrect. That's not the prototype; it's the constructor.)

Efficient implementation of algorithms

I'm sure there's a smarter way to do flood fills than to start filling in all directions all at once (there'll be overlap), and there's definitely a simpler way to count the number of adjacent mines, but I'll get to that in a bit.

Should I break some functionality down more or less?

Sort of answered in the first part: I'd add some lower-level helper methods/functions.

Is there a better way to place down mines?

Yes. Right now, since you're picking squares at random, you can end up pick the same one again and again. Theoretically, you might always pick the same one, so instead of 10 mines on the board, there'd be just 1. Really, there can be anything from 1 to n mines when you ask for n mines.

I'd instead make a 1D copy of the board, find an index with Math.floor(Math.random() * array.length) and mark it as mined it but also remove that element from the array so it can't be picked again. However, this won't work with plain numbers (without some alteration) since they're copied, not referenced - but I wouldn't use plain numbers. Which is the point the next bit.

Edit: I misread the code; it's already doing what I was suggesting :P

Alternatives

I'd use Tile (or Square or Cell) objects for the tiles instead of an array of numbers. Numbers work too, but it's just a bit abstract ("nines are mines" is a nice rhyme, though).

Also, there's no reason to have two separate boards just to track what's visible and what's not. Again, if tiles are objects, they could just have a revealed boolean property.

So something like this:

function Tile() {
  this.mined = false;
  this.revealed = false;
  this.flagged = false;
  this.count = 0;
}

So this is basically just a container. You could skip the constructor, and write it as an object literal ({mined: false, revealed: false, ...}), but with this you have the option of adding prototype methods (as explained above) later if you want you Tile objects to be more than just containers.

The main point is that the domain is being modelled more explicitly than it would be if you just use numbers. And, since objects are references, they can be passed around and manipulated easily, while an array of numbers would need to be copied everywhere.

Question is whether you want to tell the tiles how many bombs are nearby, or if you want the Tiles to have knowledge of their surroundings (i.e. know their row and column, and/or have a way of finding their neighbors). Honestly, I'm never quite sure which path to pick (both have their icky parts).

In this case I'd go with the first, though, since instead of placing mines and then iterating through all the tiles, you can simply update the surrounding tiles when you place a mine.

This isn't a complete implementation, just a few highlights you might find useful:

// create a 2D array of tiles
var board = [];
for (var r = 0; r < rows; r++) {
  var row = [];
  for (var c = 0; c < cols; c++) {
    row.push(new Tile());
  }
  board.push(row);
}

// get a 1D array of tiles (row-wise first)
var tiles = [].concat.apply([], board);

// helper function (you might also want to add a complementary
// rowColToIndex() function)
function indexToRowCol(index) {
  // the `| 0` is a bitwise trick that has the same effect as Math.floor(...)
  return {
    row: index / cols | 0,
    col: index % rows
  };
}

// place mines
var unminedTiles = tiles.slice(0); // copy the 1D array
for(var i = 0; i < mines; i++) {
  var idx = Math.random() * unminedTiles.length | 0;
  var coord = indexToRowCol(idx);

  // remove tile from unminedTiles and mine it
  unminedTiles.splice(idx, 1)[0].mined = true;

  // update tiles adjecent to the new mine
  // (neighborsOf needs to implemented, but I'll leave that to you)
  neighborsOf(coord.row, coord.col).forEach(function (tile) {
    tile.value += 1;
  });
}

You could also consider adding neighbors to the tiles. E.g. each tile has a neighbors array, and after building the board, you also go through it once, finding all neighbors for each tile, and storing them in the tile's array. Or, as mentioned, the tile could know its coordinates, and do the +1/-1 stuff itself when needed. However, it'd create a circular reference between the tiles and the board they're part of, which... I just don't like doing that.

Anyway, the point is that if the tiles know - or can find - their neighbors, it might make the flood filling easier, as you can just call flood() on the clicked tile, and then (if it's blank) it can recursively call flood on its neighbors, who (if they're blank) can call their neighbors, etc.. Not a very elegant solution, as there'll be a lot of unnecessary calls - it's basically the same solution as now, just recursive - but it should work.