1

I turned up a simple MineSweeper program i wrote several years ago and got intersted if you could optimize the following code, because that doesn't look good at all.

The following method is for checking if the indexes aren't out of range and returns the value of the requested index:

private static bool IsMineOnCoords(bool[,] field, int posI, int posJ)
{
    if (posI < 0 || posI > field.GetLength(0) - 1 || posJ < 0 || posJ > field.GetLength(1) - 1) return false;
    return field[posI, posJ];
}

I call this method for every position around a given index in 8 if statements:

int count = 0;

if (IsMineOnCoords(field, posI + 1, posJ)) count++;
if (IsMineOnCoords(field, posI - 1, posJ)) count++;
if (IsMineOnCoords(field, posI + 1, posJ + 1)) count++;
if (IsMineOnCoords(field, posI + 1, posJ - 1)) count++;
if (IsMineOnCoords(field, posI - 1, posJ + 1)) count++;
if (IsMineOnCoords(field, posI - 1, posJ - 1)) count++;
if (IsMineOnCoords(field, posI, posJ + 1)) count++;
if (IsMineOnCoords(field, posI, posJ - 1)) count++;

Is there a better way how i could do this?

Martin Niederl
  • 649
  • 14
  • 32

3 Answers3

2

Something like this should work for you. Basically, you just figure out the min and max values for the i and j coordinates of the surrounding cells, and then loop through those cells, adding up the true values (but skipping the cell that was passed in):

private static int GetSurroundingBombCount(bool[,] field, int posI, int posJ)
{
    var surroundingBombCount = 0;

    // Get the minimum and maximum i and j coordinates of the surrounding cells
    var iMin = posI - 1 < 0 ? 0 : posI - 1;
    var iMax = posI + 1 > field.GetUpperBound(0) ? posI : posI + 1;
    var jMin = posJ - 1 < 0 ? 0 : posJ - 1;
    var jMax = posJ + 1 > field.GetUpperBound(1) ? posJ : posJ + 1;

    // Loop through these cells and increment our counter for each true value
    // unless we hit the initial cell, in which case we just continue
    for (int i = iMin; i <= iMax; i++)
    {
        for (int j = jMin; j <= jMax; j++)
        {
            if (i == posI && j == posJ) continue;
            if (field[i, j]) surroundingBombCount++;
        }
    }

    return surroundingBombCount;
}

Then the usage would look something like:

int surroundingBombCount = GetSurroundingBombCount(field, posI, posJ);

I used this code to test it, which generates a 3x3 grid (you can adjust it to whatever you want), places random bombs on the cells, then displays two grids so you can visually check them: One grid with the bomb locations, and another grid with the counts:

private static void Main()
{
    bool[,] field = new bool[3, 3];

    Random rnd = new Random();

    // The lower this number, the more bombs will be placed.
    // Must be greater than one, should be greater than 2. 
    // If set to '2', all cells will contain a bomb, which isn't fun.
    int bombScarcity = 10; 

    // Assign random bombs
    for (int i = 0; i < field.GetLength(0); i++)
    {
        for (int j = 0; j < field.GetLength(1); j++)
        {
            field[i, j] = rnd.Next(1, bombScarcity) % (bombScarcity - 1) == 0;
        }
    }

    // Show bomb locations
    Console.WriteLine("\nBomb Locations:\n");

    for (int i = 0; i < field.GetLength(0); i++)
    {
        for (int j = 0; j < field.GetLength(1); j++)
        {
            Console.Write(field[i, j] ? " T" : " F");
            if ((j + 1) % field.GetLength(1) == 0) Console.WriteLine();
        }
    }

    // Show bomb counts 
    Console.WriteLine("\nBomb Counts:\n");

    for (int i = 0; i < field.GetLength(0); i++)
    {
        for (int j = 0; j < field.GetLength(1); j++)
        {
            Console.Write($" {GetSurroundingBombCount(field, i, j)}");
            if ((j + 1) % field.GetLength(1) == 0) Console.WriteLine();
        }
    }

    Console.Write("\nDone!\nPress any key to exit...");
    Console.ReadKey();
}

Output

enter image description here

And for fun a 10 x 10:

enter image description here

Rufus L
  • 36,127
  • 5
  • 30
  • 43
  • 1
    Added a "bombScarcity" variable to adjust the number of bombs (and therefore the difficulty level). Doing a random coin flip for true false was placing too many bombs in my opinion, and the user doesn't get that great feeling of clicking a cell and seeing a whole landscape of blank cells open up. :) – Rufus L Apr 18 '17 at 18:05
1

I think it is good and as far as I know it should execute fast especially because of short-circuiting.

The conditional-OR operator (||) performs a logical-OR of its bool operands. If the first operand evaluates to true, the second operand isn't evaluated. If the first operand evaluates to false, the second operator determines whether the OR expression as a whole evaluates to true or false.

https://msdn.microsoft.com/en-us/library/6373h346.aspx

DigheadsFerke
  • 307
  • 1
  • 8
0

Use something like this:

for (int i = posI - 1; i <= posI + 1; i++)
{
    for (int j = posJ - 1; j <= posJ + 1; j++)
    {
        if (!(i == posI && j == posJ) && IsMineOnCoords(field, i, j))
        {
            count++;
        }
    }
}
Uladzimir Palekh
  • 1,846
  • 13
  • 16