[C#] Stack overflow - SOLVED

[Insolence]

SOLVED


I'm messing around, porting my AutoIt OCR for Diablo II to C#, this is the PixelSearch function (derived from AutoIt)

I have NO idea why it's causing a stack overflow, if you need more code let me know

Code Select Expand

public int[][] PixelSearch(int startX, int startY, int endX, int endY, int pixel)
{

int width = endX - startX;
int height = endY - startY;
int area = width * height;

int[][] Pixels = new int[area][];

int i = 0;

for ( int x = startX; x < endX; x++ )
{
for ( int y = startY; y < endY; y++ )
{
if ( pixel == GetPixel( x, y ) )
{
Pixels[i] = new int[2] { x, y };
i++;
}
}
}

this.PixelRect = Pixels;
    return Pixels;

}

Thanks in advance

[MyndFyre]

Since you didn't post your solution, I just wanted to point out common reasons for stack overflows.  Generally, StackOverflowExceptions and OutOfMemoryExceptions in .NET are caused by using function code that directly or indirectly calls itself (recursive functions), such as:

Code Select Expand


public void CauseAnException() {
  CauseAnException();
}


This function will call itself forever until it generates an exception.

For fun, try this out:

Code Select Expand


public void CauseAnException() {
  try {
    CauseAnException();
  } catch {
    CauseAnException();
  } finally {
    CauseAnException();
  }
}

See if you can get a runtime error

[Insolence]

My problem was this:

Code Select Expand

public int PixelRectLength
{
get { return this.PixelRectLength; }
set { this.PixelRectLength = value; }
}

I have NO idea why that was failing, and I just copied it from memory.  I simply dropped the getter/setter and made it a member (right?) like so:

Code Select Expand

public int PixelRectLength;

EDIT:
I forgot to thank you for being so gratious in your explanation, thank you

[MyndFyre]

It was failing because it was referencing itself.  You need to have a private member variable, like so:

Code Select Expand


private int m_pixelRectLength;
public int PixelRectLength { get { return m_pixelRectLength; } set { m_pixelRectLength = value; } }


The way your code would work is like so:
(main code) get_PixelRectLength()
in PixelRectLength, get: get_PixelRectLength()
PixelRectLength
PixelRectLength

Forever.

That's that circular, recursive code that I mentioned in the last post.

And by making it a public variable rather than a private variable with public property, you're making it harder to update your code later.  What if at some point you need to calculate the PixelRectLength value instead of just having a base value?

[Insolence]

I'm not sure what you mean by your last statement?

I don't really need the get/set code, so aren't I fine with it the way it is?

[indulgence]

You are most likely fine with having a public variable - however its not of the object-oriented mentality of encapsulation