Search code examples
c++arraysenumspointer-to-pointerpointer-to-array

C++ Getting neighbours of a cell in a grid, -1x throwing null exception when checked if != NULL


recently moved from C# to C++ so I'm new to pointers and references and so on.

I've a pointer-to-pointer array declared like this

enum Type
{
    Void,
    DeepWater,
    Water,
    ... etc }

Tile::Type** tiles;

TileManager::TileManager(int width, int height)
{
    this->tiles = new Tile::Type*[width];

    for (int w = 0; w < width; w++)
    {
        tiles[w] = new Tile::Type[height];

        for (int h = 0; h < height; h++)
        {
            tiles[w][h] = Tile::Type::Dirt;
        }
    }
}

Now I'm putting together a method that returns the neighbours of a cell in the tiles array and checking if each neighbour is not-equal to NULL. However even when checking whether it's null or not seems to throw an error, so I'm stumped.

Tile::Type * TileManager::GetNeighbours(int x, int y)
{
    Tile::Type neighbours[8];

    if(tiles[x][y+1] != NULL)
        neighbours[0] = tiles[x    ][y + 1];

    ...etc

    if (tiles[x - 1][y - 1] != NULL)    //<-- Error fires here
        neighbours[5] = tiles[x - 1][y - 1];

    return neighbours;
}

I know why it's throwing the error but shy of checking X and Y to see if they go over the limit or below 0... I figure there's a more practical way to prevent this so thought I'd best ask.

Edit:

Thank you, user4581301. I found most of this code elsewhere and adapted it to reflect the changes you suggested.

std::array<Tile::Type, 8> TileManager::GetNeighbours(int c, int r)
{
    std::array<Tile::Type, 8> neighbours;

    const int y[] = { -1, -1, -1,  1, 1, 1,  0, 0 };// 8 shifts to neighbors
    const int x[] = { -1,  0,  1, -1, 0, 1, -1, 1 };// used in functions 

    for (int i = 0; i < 8; ++i)// visit the 8 spaces around it
        if (inField(r + y[i], c + x[i]))
            neighbours[i] = tiles[r + y[i]][c + x[i]];
        else
            neighbours[i] = Tile::Type::Void;

    return neighbours;
}

bool TileManager::inField(int r, int c)
{
    if (r < 0 || r >= 25) return false;
    if (c < 0 || c >= 25) return false;
    return true;
}

Solution

  • tiles[x][y+1], if y is the maximum valid value, will not be NULL except by the grace of . This goes out of bounds and as soon as you go out of bounds all bets are off. You've invoked Undefined Behaviour and pretty much anything can happen. Even what you expected to happen.

    The same applies to the reported crash site, tiles[x - 1][y - 1].

    Edit: Left out solution. Not helpful.

    The only way, short of taking off and nuking the entire site from orbit, is to test the index to make sure it does not puncture the array bounds before using the index on the array. You'll probably want a function to handle this.

    void assign_if(Type & neighbour, int x, int y)
    {
        if(x >= 0 && x < width && y >= 0 && y < height)
        neighbour = tiles[x][y];
    }
    

    and call it

    assign_if(neighbours[0], x, y+1);
    

    and later

    assign_if(neighbours[0], x-1, y-1);
    

    Edit: Stealing this from Bob__ for completeness

    It is impossible to return a raw array from a function. The array goes out of scope and the pointer to it becomes invalid. Either pass in the array as another parameter or use a std::array or std::vector, both of which can be returned. Thanks to Copy Elision, a smart compiler will likely eliminate the copying costs.

    Example:

    std::array<Tile::Type, 8> TileManager::GetNeighbours(int x, int y)
    {
        std::array<Tile::Type, 8> neighbours;
        ...
        return neighbours;
    }
    

    Edit by original poster. Here is my solution:

    std::array<Tile::Type, 8> TileManager::GetNeighbours(int c, int r)
    {
        std::array<Tile::Type, 8> neighbours;
    
        const int y[] = { -1, -1, -1,  1, 1, 1,  0, 0 };// 8 shifts to neighbors
        const int x[] = { -1,  0,  1, -1, 0, 1, -1, 1 };// used in functions 
    
        for (int i = 0; i < 8; ++i)// visit the 8 spaces around it
            if (inField(r + y[i], c + x[i]))
                neighbours[i] = tiles[r + y[i]][c + x[i]];
            else
                neighbours[i] = Tile::Type::Void;
    
        return neighbours;
    }
    
    bool TileManager::inField(int r, int c)
    {
        if (r < 0 || r >= 25) return false;
        if (c < 0 || c >= 25) return false;
        return true;
    }
    

    Edit: Caveat

    This answer deals directly with solving the problem as asked. See the answer by Kaz for a description of a more practical solution that trades a bit of memory to completely eliminate the need for testing and generating the neighbours array.