Why does this button object cause a stack overflow?

Question

 void OpenClean( int i, int j) {

        bnt[i, j] = new Button();
        if (!bnt[i,j].Enabled) return;

        bnt[i, j].Enabled = false;
        bnt[i,j].BackColor = Color.Aquamarine;
        bnt[i, j].Text = data[i, j].ToString();
        if (data[i, j] == 0) {

            if (i > 0 && j > 0 && data[i - 1, j - 1] == 0) OpenClean(i - 1, j - 1);
            if (i > 0 && data[i - 1, j] == 0) OpenClean(i - 1, j);
            if (i > 0 && j < 8 && data[i - 1, j + 1] == 0) OpenClean(i - 1, j + 1);
            if (j > 0 && data[i, j - 1] == 0) OpenClean(i, j - 1);
            if (j < 8 && data[i, j + 1] == 0) OpenClean(i, j + 1);
            if (i < 8 && j > 0 && data[i + 1, j - 1] == 0) OpenClean(i + 1, j - 1);
            if (i < 8 && data[i + 1, j] == 0) OpenClean(i + 1, j);
            if (i < 8 && j < 8 && data[i + 1, j + 1] == 0) OpenClean(i + 1, j + 1);
        }

It tells me Process is terminated due to StackOverflowException. Because of my limited experience,I didn't find an effective way. This question has been bothering me for a long time. Please help or try to give me some ideas how to solve it.

Thanks in advance!

You are calling your `OpenClean` method recusively. I didn't check all your conditions, (and I'm not able to, because I don't know the contents of the `data` array) but probably you have an error in your conditions, so your recursive calling never stops. Which after some time results in a StackOverflow exception — derpirscher, May 30 '21 at 15:17
Does this answer your question? [How do I prevent and/or handle a StackOverflowException?](https://stackoverflow.com/questions/206820/how-do-i-prevent-and-or-handle-a-stackoverflowexception) and [Stack overflow error in C# - but how to fix it?](https://stackoverflow.com/questions/4033805/stack-overflow-error-in-c-sharp-but-how-to-fix-it) — , May 30 '21 at 15:17
This is a good time to learn how to use the debugger. Put a breakpoint on the method. When the program stops, look at the _call stack_ write down what the parameters are. Step, step, step. Keep stepping till you call the method again (you'll then hit your breakpoint again). It should become obvious what's happening — Flydog57, May 30 '21 at 15:42

Olivier Jacot-Descombes · Answer 1 · 2021-05-30T15:54:04.483

You never set data[i, j] to a value other than 0. Therefore, the recursion never stops. You must set the inspected cells to a different value so that they will not get inspected again and again and again ...

You also create a new button with bnt[i, j] = new Button(); instead of inspecting the existing button. Therefore you will never see a button with Enabled == false, since the default value for Enabled is true.

It is better to base the logic on the model (i.e. data[,]) and to use the UI (the controls) only for display.

const int Inspected = -1;

void OpenClean( int i, int j) {
    ...
    if (data[i, j] == 0) {
        data[i, j] = Inspected; // <====== This stops the endless recursion!
        if (i > 0 && j > 0 && data[i - 1, j - 1] == 0) OpenClean(i - 1, j - 1);
        ...
    }
}

And, btw., you are testing for 0 twice. Before calling OpenClean and inside OpenClean again. Just do it once.

void OpenClean( int i, int j) {
    if (data[i, j] == 0) { // This one does the job.
        data[i, j] = Inspected; // <======
        // bnt[i, j] = new Button();  <=== drop this!
        // if (!bnt[i,j].Enabled) return;  <=== drop this! We test data instead.

        bnt[i, j].Enabled = false;
        bnt[i,j].BackColor = Color.Aquamarine;
        bnt[i, j].Text = data[i, j].ToString();

        if (i > 0 && j > 0) OpenClean(i - 1, j - 1);
        if (i > 0         ) OpenClean(i - 1, j);
        if (i > 0 && j < 8) OpenClean(i - 1, j + 1);
        if (         j > 0) OpenClean(i,     j - 1);
        if (         j < 8) OpenClean(i,     j + 1);
        if (i < 8 && j > 0) OpenClean(i + 1, j - 1);
        if (i < 8         ) OpenClean(i + 1, j);
        if (i < 8 && j < 8) OpenClean(i + 1, j + 1);
    }
}

If you do the range test at the beginning of OpenClean, you can get rid of all the other range tests.

void OpenClean( int i, int j) {
    if (i > 0 && i < 8 && j > 0 && j < 8 && data[i, j] == 0) { // This one does the job.
        data[i, j] = Inspected; // <======

        bnt[i, j].Enabled = false;
        bnt[i,j].BackColor = Color.Aquamarine;
        bnt[i, j].Text = data[i, j].ToString();

        OpenClean(i - 1, j - 1);
        OpenClean(i - 1, j);
        OpenClean(i - 1, j + 1);
        OpenClean(i,     j - 1);
        OpenClean(i,     j + 1);
        OpenClean(i + 1, j - 1);
        OpenClean(i + 1, j);
        OpenClean(i + 1, j + 1);
    }
}

Using the C# 9.0 patterns, you can simplify the condition somewhat

if (i is > 0 and < 8 && j is > 0 and < 8 && data[i, j] == 0)

Why does this button object cause a stack overflow?

1 Answers1