Make sure that my for loop generates exactly 8 unique number pairs

Question

It's a minesweeper game. The objective here is to generate the exact number of mines. In this case I have an 8x8 grid of boxes with 10 mines (see at the bottom). So, the nested for loop generates 64 objects with x and y coordinates (for later drawing the grid) and a state property to indicate whether the field is mined. Then, in generateBombs I generate 10 objects with state:mined with random x and y to overwrite 8 of the 64 objects in the boxes array randomly and thus plant the mines. The problem with my approach here is that there is a possibility of 2 non-unique pairs of x and y objects to be generated, and this way I'll end up with less than the original number of mines, because the same object will be overwritten twice. What is a good approach here?

Also, one of my requirements is for the generator to use a helper function for the mines, but they take the same arguments, the need might be defeated.

var minesweeper = {
  boxes: [],
  //rows
  boxesNum: 0,
  bombsNum: 0,

  //creates a 8x8 grid 
  generateMap: function (width, height, bombsNum) {
    for (i = 1; i < height; i++) {
        this.boxes.push({
            x: i,
            y: 1,
            state: "safe"
        });
        for (j = 1; j < width; j++) {
            this.boxes.push({
                x: 1,
                y: j,
                state: "safe"
            });
        }
    }
    this.generateBombs(width, height, bombsNum)
  },
  //mines random fields from the grid
  generateBombs: function (width, height, bombsNum) {
      for (k = 0; k < bombsNum; k++) {
          this.boxes.push({
             x: Math.floor(Math.random() * width + 1),
             y: Math.floor(Math.random() * height + 1),
             state: "mined"
          });
      }
   }
}

minesweeper.generateMap(8, 8, 10);

This issue is mostly language-independent, I recommend this discussion: [creating random numbers without duplicates](http://stackoverflow.com/questions/4040001/creating-random-numbers-with-no-duplicates) — doldt, Jul 14 '15 at 09:16
Just create them while you generate the map. Not afterwards. Would be much simpler, imo. — Sebastian Nette, Jul 14 '15 at 09:19
I think you will have to keep track of the coordinates on which you generated a mine. Then, upon generating a new mine, check if the coordinates were already used, if so, skip. — Veverke, Jul 14 '15 at 09:19
As said before by @doldt, generate an array containing the 64 available positions. Then use this algorithm (http://stackoverflow.com/questions/9719434/picking-2-random-elements-from-array#answer-9719618) to select your 10 random positions. — romainsalles, Jul 14 '15 at 09:22
Guys, there's no need to overcomplicate this. Just grab a random cell, "bomb" it if it is safe, repeat 'till you have 10 bombs. — Cerbrus, Jul 14 '15 at 09:24

Cerbrus · Accepted Answer · 2015-07-14T09:36:23.157

1

You'd be better off working on the array of boxes itself, rather than generating bombs first.

generateBombs: function (width, height, bombsNum) {
    var bombCount = 0;                          // Count how many bombs we planted,
    while(bombCount < 10){                      // Loop until we have 10 bombs,
        var index = parseInt(Math.random() * this.boxes.length + 1); // Get a random box id,
        if(this.boxes[index].state === "safe"){ // If the box is safe, plant a bomb.
            this.boxes[index].state = "mined";
            bombCount++;                        // Increase bomb count with 1.
        }
    }
}

This method will guarantee that you have 10 bombs planted at 10 different locations. It could select the same box twice, but if it does so, it just tries again.

In the best case scenario, you have 10 iterations of the loop, compared to other methods that require arrays of coordinates to be checked for every bomb you want to generate.

However, there's one problem with this method of randomly picking a bomb position:
If you increase the bomb count, the method will hit more and more boxes where bombs already have been planted, resulting in an exponential increase of iterations required to plant as many bombs as you want. Basically, the denser the field is, the more likely the function is to randomly select a cell that already has a bomb, so it'd have to try again.

I don't expect this to be noticeable at bomb counts of, say, 50% or lower, though.

Your generateMap function is also broken. Try this instead:

generateMap: function (width, height, bombsNum) {
    for (var i = 0; i < width; i++) {
        for (var j = 0; j < height; j++) {
            this.boxes.push({
                x: (i + 1),
                y: (j + 1),
                state: "safe"
            });
        }
    }
    this.generateBombs(width, height, bombsNum)
},

edited Jul 14 '15 at 09:36

answered Jul 14 '15 at 09:21

Cerbrus

70,800
18
132
147

the nested for is crashing my console for some reason. Your approach is excellent, though. – Svetan Dimoff Jul 14 '15 at 09:43
Yea, I accidentally had a infinite loop in there, fixed that a moment ago. – Cerbrus Jul 14 '15 at 09:44
Is it a good idea to give the function access to outside data structures (this.boxes) and variables? Should it stay encapsulated instead? – Svetan Dimoff Jul 14 '15 at 09:53
How else would you do it? This seems fine to me. – Cerbrus Jul 14 '15 at 09:58
No idea, but functions should be encapsulated and working with their own variables, arrays. Maybe it should be passed the array from the `generateMap`. But then again, after it works wit hit, it has to write to the global one, still. – Svetan Dimoff Jul 14 '15 at 10:09
Passing the `boxes` array could work, but I don't really see the point: `this.generateBombs(width, height, bombsNum, this.boxes)` – Cerbrus Jul 14 '15 at 10:10
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/83185/discussion-between-stevens-haen-and-cerbrus). – Svetan Dimoff Jul 14 '15 at 10:10
`generateMap` should be passed the global array in the first place (not at the declaration but rather the invocation, I have an additional object that invokes the functions), then work with it with own argument variable, then pass it to `generateBombs`. Then generate bombs may return it's own, I don't know how it should be written in the object property. – Svetan Dimoff Jul 14 '15 at 10:12
Oh, so the code in the question is more of an working _example_ for those 2 functions. – Cerbrus Jul 14 '15 at 10:16

Make sure that my for loop generates exactly 8 unique number pairs

1 Answers1