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;
    }

Which line in the code is highlighted when you receive the exception? — Keith Payne, Jan 20 '14 at 13:03
For the love of Cthulhu, please stop this i,j,k,x nonsense and use meaningful identifier names. — asawyer, Jan 20 '14 at 13:05
On the other hand, you are constantly using the `++` and `--` operators. Are you aware that they actually change the value of the variables? I think instead, you may have to use `mat[(i + 1),j]` for example. — John Willemse, Jan 20 '14 at 13:06
i and j curent values of the position. n and m the max values of the array — user3215160, Jan 20 '14 at 13:07
Example: let `int j = 3`. Now, after `if (j++ < m)...` and `if (mat[j++, i]...` the value of `j` will be 5. Is that what you expect? — John Willemse, Jan 20 '14 at 13:13
no i expect to be j=4 if j=3 i modified as you said and is working. I'm still new to c#. Thanks. — user3215160, Jan 20 '14 at 13:17

score 4 · Accepted Answer · answered Jan 20 '14 at 13:29

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.

Also I may have the width and height back to front there... Can't quite recall which is which. — Rhys van der Waerden, Jan 20 '14 at 13:33
Thank you for your help this really helped me simplify the source code and get the positions i needed. Thank you again. — user3215160, Jan 21 '14 at 17:26

C# Check for neighbours

1 Answers1