C# Check for neighbours

Question

I have a function to check neighbors of an array and if that element is equal with 1. X is for each neighbor found and v[l] is the position for each 0. I have a problem with this code each time gives me "Index was outside the bounds of the array" and i don't know what to do else.

 public int modificari(int i,int j,int n,int m)
    {
        int x = 0;
        v = new int[n];
        l=0;
        if (mat[i, j] == 1)
        {
            if (j++ < m)
            {
                if (mat[i, j++] == 1)
                    x++;
                else
                {
                    v[l] = i * n + j + 2;
                    l++;
                }
            }
            if (j++ < m && i++ < n)
            {
                if (mat[i++, j++] == 1)
                    x++;
                else
                {
                    v[l] = (i + 1) * n + j + 2;
                    l++;
                }
            }
            if (i++ < n)
            {
                if (mat[i++, j] == 1)
                    x++;
                else
                {
                    v[l] = (i + 1) * n + j + 1;
                    l++;
                }
            }
            if (j-- >= 0 && i++ < n)
            {
                if (mat[i++, j--] == 1)
                    x++;
                else
                {
                    v[l] = (i + 1) * n + j;
                    l++;
                }
            }
            if (j-- >= 0)
            {
                if (mat[i, j--] == 1)
                    x++;
                else
                {
                    v[l] = i * n + j;
                    l++;
                }
            }
            if (j-- >= 0 && i-- >= 0)
            {
                if (mat[i--, j--] == 1)
                    x++;
                else
                {
                    v[l] = (i - 1) * n + j;
                    l++;
                }
            }
            if (i-- >= 0)
            {
                if (mat[i--, j] == 1)
                    x++;
                else
                {
                    v[l] = (i - 1) * n + j + 1;
                    l++;
                }
            }
            if (j < n && i-- >= 0)
            {
                if (mat[i--, j++] == 1)
                    x++;
                else
                {
                    v[l] = (i - 1) * n + j + 2;
                    l++;
                }
            }
            if (x < 2 && x > 3)
                return 1;
            else
                return random();
        }
        return x;
    }

Answer 1

That is a total mess. It is very hard to follow, even for an experienced coder. Use of one letter variable names and inline ++ operators is usually discouraged for the sake of readability.

I've quickly tried to rewrite your function from my best guess of what you're trying to achieve. I'm hoping you can see a different way to approach the problem that suits you better.

NOTE: I did not test this code at all, it probably has compile errors.

public struct Point
{
    public int X;
    public int Y;
    public Point( int x, int y )
    {
        X = x;
        Y = y;
    }
}

public class Whatever
{
    // ...

    // Here is a list of the positions of all the neighbours whose values are
    // zero.
    List<Point> zeroPositions = new List<Point>();

    // ...

    public int Modificari(int pointX, int pointY)
    {
        // Determine dimensions of array.
        int height = mat.GetLength(0);
        int width = mat.GetLength(1);

        // Find the minimum and maximum positions bounded by array size. (So we
        // don't try to look at cell (-1, -1) when considering the neighbours of
        // cell (0, 0) for instance.
        int left = Math.Max( pointX - 1, 0 );
        int right = Math.Min( pointX + 1, width );

        int top = Math.Max( pointY - 1, 0 );
        int bottom = Math.Min( pointY + 1, height );

        // This is the number of neighbours whose value is 1.
        int oneCount = 0;

        zeroPositions.Clear();

        for( int y = top; y <= bottom; y++ )
        {
            for( int x = left; x <= right; x++ )
            {
                if( mat[x, y] == 1 )
                {
                    oneCount++;
                }
                else if( mat[x, y] == 0 )
                {
                    zeroPositions.Add( new Point( x, y ) );
                }
            }
        }

        return oneCount;
    }

    //...
}

Also I'd really advise you to try not to do too many things in a function. Try making a different function for getting positions of ones and for returning the number of zeros.