-4

I keep getting a stack overflow error but I can't figure out why or how to fix it. Here is my code:

 private void ShowDiff(int leftIndex, int rightIndex)
        {
            if (leftIndex > 0 && rightIndex > 0 &&
                _compareFunc(_left[_preSkip + leftIndex - 1], _right[_preSkip + rightIndex - 1]))
            {
                ShowDiff(leftIndex - 1, rightIndex - 1);
                FireLineUpdate(DiffType.None, _preSkip + leftIndex - 1, -1);
            }
            else
            {
                if (rightIndex > 0 &&
                    (leftIndex == 0 ||
                     _matrix[leftIndex, rightIndex - 1] >= _matrix[leftIndex - 1, rightIndex]))
                {
                    ShowDiff(leftIndex, rightIndex - 1);
                    FireLineUpdate(DiffType.Inserted, -1, _preSkip + rightIndex - 1);
                }
                else if (leftIndex > 0 &&
                         (rightIndex == 0 ||
                          _matrix[leftIndex, rightIndex - 1] < _matrix[leftIndex - 1, rightIndex]))
                {
                    ShowDiff(leftIndex - 1, rightIndex);
                    FireLineUpdate(DiffType.Deleted, _preSkip + leftIndex - 1, -1);
                }
            }

        }

This is the error I keep getting:

An unhandled exception of type 'System.StackOverflowException' occurred in Comparer.exe

Any help is appreciated.

mikestram
  • 183
  • 2
  • 9
  • 2
    You need to carefully evaluate how the recursiveness of this method behaves, such that you will always end. Could it be that you're giving it a large piece of data so that even if it works exactly as it should, it's going to consume more than the default stack size which is 1MB? – Lasse V. Karlsen Jan 29 '15 at 14:03
  • 4
    Did you try stepping through it, or using debug prints to see what is happening? – pquest Jan 29 '15 at 14:03
  • It is a classical infinite loop. As suggested by @pquest, print the values before your compare function and where you call your recursive point (1st, 2nd or 3rd call). You will figure out easily where your loop is being infinite. – rodrigogq Jan 29 '15 at 14:05
  • Another question that will help you debug your issue http://stackoverflow.com/questions/206820/how-do-i-prevent-and-or-handle-a-stackoverflowexception –  Jan 29 '15 at 14:05
  • That seems like a very poor candidate for a duplicate link for this question? The answer answers a *very specific* stack overflow problem, and does not in the least way possible describe how to generally track down a stack overflow problem. – Lasse V. Karlsen Jan 29 '15 at 14:06
  • How large arrays are you comparing? The function will call itself as deep as there are items to compare. If you have many items you will simply run out of stack space. You should use a loop instead if you want to compare large arrays. – Guffa Jan 29 '15 at 14:08
  • essentially `ShowDiff` calls `ShowDiff` too much. – Jodrell Jan 29 '15 at 14:11
  • @rodigogq doesn't look like a infinite loop to me or even infinitely recursive. – Jodrell Jan 29 '15 at 14:19
  • Hi thanks all for your help. More info: so when I call the method leftIndex = 15896 and rightindex = 14696. When I get the error leftindex = 14062 and rightindex = 13002. I've tested this on smaller data (around 300 for left and right) and it worked fine. So I guess I'm running out of stack space like Guffa and Lasse suggested. Would using a loop instead of a recursive method resolve the problem? – mikestram Jan 29 '15 at 14:33
  • @LasseV.Karlsen okay, then, close it as dupe of this one http://stackoverflow.com/questions/206820/how-do-i-prevent-and-or-handle-a-stackoverflowexception he needs to debug his SOE. He hasn't done that. Unless you're into debugging people's code for them? –  Jan 29 '15 at 14:35
  • Thanks Jodrell! Using a loop fixed the stack overflow error. For some reason your code isn't working properly though, it's comparing the list elements out of order. I will debug and look into this but thanks again! – mikestram Jan 29 '15 at 14:52

1 Answers1

1

EDIT: Added break, in case no condition is true.


Instead, rewrite the function using a loop. Additionally, you can simplify your logic somewhat.

Note: the assumption that both leftIndex and rightIndex are greater than 0 persists.

 private void ShowDiff(int leftIndex, int rightIndex)
 {
     while(leftIndex > 0 && rightIndex > 0)
     {
         if (leftIndex > 0 && rightIndex > 0 &&
                _compareFunc(
                    _left[_preSkip + leftIndex - 1],
                    _right[_preSkip + rightIndex - 1]))
         {
             leftIndex--;
             rightIndex--;
             FireLineUpdate(
                 DiffType.None,
                 _preSkip + leftIndex - 1,
                 -1);
         }
         else
         {
             if (rightIndex > 0 &&
                    (_matrix[leftIndex, rightIndex - 1] >=
                        _matrix[leftIndex - 1, rightIndex]))
             {
                 rightIndex--;
                 FireLineUpdate(
                     DiffType.Inserted,
                     -1,
                     _preSkip + rightIndex - 1);
             }
             else if (_matrix[leftIndex, rightIndex - 1] <
                         _matrix[leftIndex - 1, rightIndex]))
             {
                 leftIndex--;
                 FireLineUpdate(
                     DiffType.Deleted,
                     _preSkip + leftIndex - 1,
                     -1);
             }
             else
             {
                 break;
             }
         }
     }
 }
Jodrell
  • 34,946
  • 5
  • 87
  • 124