1

I'm currently making a minesweeper game on C++, but there seems to be a bug. There's a function I wrote to return the amount of mines around the spot, just like in a classic game of minesweeper. However, the code fails near the end. I have provided some code to better explain.

int findBeginnerMines(string actualBoard[BEGINNER_DIMENSION][BEGINNER_DIMENSION], int x, int y)
{
    int mines = 0;
    
    if (actualBoard[x][y] == MINE)
    {
        mines = -1;
    }
    
    else
    {
        if (actualBoard[x - 1][y - 1] == MINE)
        {
            mines++;
        }
        
        if (actualBoard[x][y - 1] == MINE)
        {
            mines++;
        }
        
        if (actualBoard[x + 1][y - 1] == MINE && x != BEGINNER_DIMENSION - 1)
        {
            mines++;
        }
        
        if (actualBoard[x - 1][y] == MINE)
        {
            mines++;
        }
        
        if (actualBoard[x + 1][y] == MINE && x != BEGINNER_DIMENSION - 1)
        {
            mines++;
        }
        
        if (actualBoard[x - 1][y + 1] == MINE && y != BEGINNER_DIMENSION - 1)
        {
            mines++;
        }
        
        if (actualBoard[x][y + 1] == MINE && y != BEGINNER_DIMENSION - 1)
        {
            mines++;
        }
        
        if (actualBoard[x + 1][y + 1] == MINE && x != BEGINNER_DIMENSION - 1 && y != BEGINNER_DIMENSION - 1)
        {
            mines++;
        }
    }
    return mines;
}

In this case, BEGINNER_DIMENSION = 8. When x = 7, this function returns an error.

Mureinik
  • 297,002
  • 52
  • 306
  • 350
  • 8
    `if (actualBoard[x + 1][y - 1] == MINE && x != BEGINNER_DIMENSION - 1)` It's kinda pointless to check for index-out-of-bounds **after** you've already accessed it and triggered undefined behavior. Check whether the index is valid before actually using the same. – Igor Tandetnik Dec 27 '20 at 23:18
  • Should all these conditions in the `else` block really be or'ed, or did you rather want to make a `if() { } else if() {}` cascade there? – πάντα ῥεῖ Dec 27 '20 at 23:19
  • 1
    Related reading to help explain your problem: [Is short-circuiting logical operators mandated? And evaluation order?](https://stackoverflow.com/q/628526/2602718) – scohe001 Dec 27 '20 at 23:19

1 Answers1

4

You access the array before checking that the values of x and y allow such access. I.e., you need to flip the expressions around. E.g., replace:

if (actualBoard[x + 1][y - 1] == MINE && x != BEGINNER_DIMENSION - 1)

with

if (x != BEGINNER_DIMENSION - 1 && actualBoard[x + 1][y - 1] == MINE)
Mureinik
  • 297,002
  • 52
  • 306
  • 350
  • 2
    Valuable note on the subject: "_Builtin operators && and || perform short-circuit evaluation (do not evaluate the second operand if the result is known after evaluating the first)_ - [cppreference](https://en.cppreference.com/w/cpp/language/operator_logical) – Ted Lyngmo Dec 27 '20 at 23:29
  • 1
    @TedLyngmo Tru' dat! – Mureinik Dec 27 '20 at 23:30
  • Thanks for the reply! I was just wondering, why does the order always have to matter? Since I'm using the &&, if one fails, shouldn't it simply just not enter the if statement? Sorry if this was a bad question, I'm pretty new to programming. –  Dec 28 '20 at 19:02
  • Accessing an out-of-range element of an array will have an undefined behavior - it may return random junk from the memory, it may just crash. If you have it as the left hand operand of an `&&`, it won't get short-circuited out, and you may still suffer the crash – Mureinik Dec 28 '20 at 20:58