0

I'm trying to make a class that will check the number of mines around the tile that's checked. But the problem is that my range is set from 1-max height. if PosY and posX is 1 and 1 (Checking the tile at the top left corner), it will check in to Board array for example Board[0,0] but it's out of range, but since we don't have any tiles to check on left, top, and diagonally left to it, it would give an exception that it's out of range.

public int GetMineCount(int posY, int posX)
        {
            int count = 0;
            if (Board[posY-1, posX-1] == CellState.Mine)
                count++;
            if (Board[posY, posX-1] == CellState.Mine)
                count++;
            if (Board[posY + 1, posX - 1] == CellState.Mine)
                count++;

            if (Board[posY + 1, posX] == CellState.Mine)
                count++;

            if (Board[posY + 1, posX + 1] == CellState.Mine)
                count++;
            if (Board[posY, posX+1] == CellState.Mine)
                count++;

            if (Board[posY-1, posX+1] == CellState.Mine)
                count++;

            if (Board[posY-1, posX] == CellState.Mine)
                count++;

            return count;


        }

Is there a good practice so if the array is our of range, it would not count it.

  • why would Board[0,0] be out of range? IMHO you are looking at wrong place for the error. Did you check in debugger what were the values when you had the error. – Cetin Basoz Nov 09 '19 at 18:24
  • Best thing to do would be to check the value of the position beforehand so you can prevent the exception from occurring. But as Cetin says, 0,0 should not be considered out of range in an array – ADyson Nov 09 '19 at 18:25
  • 1
    Possible duplicate of [What is an IndexOutOfRangeException / ArgumentOutOfRangeException and how do I fix it?](https://stackoverflow.com/questions/20940979/what-is-an-indexoutofrangeexception-argumentoutofrangeexception-and-how-do-i-f) –  Nov 09 '19 at 18:27

2 Answers2

1

If the input is 0, 0, this line:

if (Board[posY-1, posX-1] == CellState.Mine)

will try ot access Index -1, -1. Wich by definition can never be within the range.

You are facing the age old problem of making a playboard and looking at all the neighbours, without acidentally looking outside the board. I can think of only a few ways around this:

  1. Add a 1 elemet thick layer of borderspace that you do not display. Mark it properly. Account for it in code. But just omit it during output.
  2. Make functions to access each direction. That function will then deal with the incorrect indexes. However this is a lot of writing work. And the copying can make it hard to read.
  3. I thought of a 3rd way. A single Function with the sole purpose of verifying if the indexes are valid. Both indexes must be >= 0, but < Count (in this dimension). This is actually a pretty standart thing to do.
Christopher
  • 9,634
  • 2
  • 17
  • 31
0

My personal preference would be making the array two rows and two columns wider, so you would get a border around the board. So a board of 10x16 mines would actually be an array of 12x18. All (0,Y), (11,Y), (X, 0) and (X, 17) values would then hold CellState.Border, meaning that you don't check if that cell has any mines. Your active board would be from (1,1) to (10, 16) which is also nice in your coordinate system!
It does mean that you'd have to check if Board[X,Y] is set to CellState.Border, though. If yes, then don't count mines! Ignore it... Otherwise, values of X-1, X+1, Y-1 and Y+1 would always be in range as those values would go from (0,0) to (11,17).
It's a simple trick that allows you to avoid adding if-statements. Of course, you could always check if the values you're asking for are within the range or not.
My nutty mind is now thinking about a way to avoid the many if-statements that you're already using. But as I don't know how experienced you are with C#, I'm avoiding any more elegant solutions that would also be a bit complex to understand. However, I suggest that you take a slightly different approach!
You see, you're calculating the mine count for each cell. However, your array could already hold the correct mine counts once you place the mines! You are placing an N amount of mines on the board so for every mine you place, you immediately increase the mine count for all surrounding cells. This means that you don't need any if-statements to check the count. You already know the count! But it also means the Board array needs a bit more data than just a CellState value. You could create a Cell class that has a CellState property, a MineCount property and a Visited property. So initially, all cells would have a MineCount value of 0 and for every mine you place, all surrounding cells get their MineCount increased by one.
Won't solve the range exception, though. For that, you actually have to check if coordinates are within the range or not, or add the border around the array as I've mentioned.


As I said, I don't know how experienced you are, but for a more advanced solution, you might want to create a function:

public CellState GetCellState(int posY, int posX)
{
    if (posX < 0) return CellState.Invalid;
    if (posY < 0) return CellState.Invalid;
    if (posX >= MaxX) return CellState.Invalid;
    if (posY >= MaxY) return CellState.Invalid;
    return Board[posY, posX];
}

This assumes you have the width and height of the board stored in the properties MaxY and MaxX. But this would allow you to rewrite your code as:

public int GetMineCount(int posY, int posX)
{
    int count = 0;
    if (GetCellState(posY-1, posX-1) == CellState.Mine)
        count++;
    if (GetCellState(posY-1, posX) == CellState.Mine)
        count++;
    if (GetCellState(posY-1, posX+1) == CellState.Mine)
        count++;
    if (GetCellState(posY, posX-1) == CellState.Mine)
        count++;
    if (GetCellState(posY, posX+1) == CellState.Mine)
        count++;
    if (GetCellState(posY+1, posX-1) == CellState.Mine)
        count++;
    if (GetCellState(posY+1, posX) == CellState.Mine)
        count++;
    if (GetCellState(posY+1, posX+1) == CellState.Mine)
        count++;
    return count;
}

Now the single function would check if each cell is within the proper range or not. And those four if-statements could be consolidated into one, but I wrote it this way for clarity. Your CellState enumeration would need to contain an "Invalid" value now, though.
It would still not be my preferred method, though. But the solution I'm thinking off is a bit complex to explain and would not involve an array... And I would calculate MineCount when I place mines, not afterwards...

Wim ten Brink
  • 25,901
  • 20
  • 83
  • 149