1

I'm in need of some assistance regarding c++ and objects\references\pointers. I'm writing a simple Chess program that has several classes. In particular I have a problem with the following two classes Board and Cell:

The board has a vector of vectors of Cells (basically a matrix). Each cell holds a pointer to the current piece that is occupying it.

class Board
{
public:
    Board();
    void drawBoard();
    bool makeMove(int startRow, int startCol, int destRow, int destCol);
private:
    vector< vector<Cell> > _board;
    void initBoard();
    void initPieces();
};

Board::Board()
{
    initBoard();
}

void Board::initBoard()
{

    for (int i = 0; i < BOARD_SIZE; i++)
    {
        vector<Cell> row; //create empty row
        for (int j = 0; j < BOARD_SIZE; j++)
        {
            row.push_back((Cell(i, j)));
        }
        _board.push_back(row);
    }

    initPieces();
}

void Board::initPieces()
{
    //Set Pawns
    for (int i = 0; i < BOARD_SIZE; i++)
    {
        _board[1][i].setOccupying(new Pawn(White));
        _board[6][i].setOccupying(new Pawn(Black));
    }
}

void Board::drawBoard()
{

    drawLettersCoord();
    for (int i = BOARD_SIZE-1; i >= 0; i--)
    {
        std::cout << i + 1 << " ";
        for (int j = 0; j < BOARD_SIZE; j++)
        {       
            _board[i][j].draw();
        }
        std::cout << " " << i + 1 << std::endl;
    }
    std::cout << "\n";

}

bool Board::makeMove(int startRow, int startCol, int destRow, int destCol)
{
    _board[startRow][startCol].getOccupyingPiece()->isLegalMove(
        startRow, startCol, destRow, destCol);
    return true;
}

here is the Cell class:

class Cell
{
public:
    Cell();
    Cell(int row, int col);
    Cell(int row, int col, ChessPiece * occupying);
    void draw();
    ChessPiece * getOccupyingPiece();
    void setOccupying(ChessPiece *occupying);
private:
    int _row;
    int _col;
    bool _occupied;
    ChessPiece * _pieceOccupying;
};

Cell::Cell()
    :Cell(0, 0)
{
    setColour();
}

Cell::Cell(int row, int col)
    : _row(row), _col(col), _occupied(false)
{
    setColour();
}

Cell::Cell(int row, int col, ChessPiece * occupying)
    : _row(row), _col(col), _pieceOccupying(occupying), _occupied(true)
{
    setColour();
}

void Cell::setOccupying(ChessPiece *occupying)
{
    _occupied = true;
    _pieceOccupying = occupying;
}
void Cell::draw()
{

    int foregroundText;
    if (!_occupied)
    {
        foregroundText = 37;
    }
    else
    {
        foregroundText = _pieceOccupying->getPlayer();
    }
    cout << "\33[" << foregroundText << ";" << _colour << "m"
         << (_occupied ? _pieceOccupying->getPieceCode(): " ") << "\33[0m";
}

ChessPiece * Cell::getOccupyingPiece()
{
    return _pieceOccupying;
}

When I run the game, I create and draw a new Board by calling

Board _board;
_board.drawBoard();

The drawing seems to work OK without any issues.

However, when I call

_board.makeMove(1,0,2,0);

to check if the pawn can make such a move, I get a memory error. Upon debugging I see that the Cell object which is being called has junk in it and not actual data. When I try to look at the pointer to the occupying piece it shows "Unable To Read Memory", so when I call the occupying piece's isLegalMove function, it crashes.

I can't seem to find out what the problem is. I don't understand why a cell inside the vector would have junk in it, it is defined in the class (without new) so as per my understanding it should be available so long as the current instance of the board is alive.

Can anyone shed some light as to what I am doing wrong?

user475680
  • 679
  • 1
  • 9
  • 21

1 Answers1

5
  1. Not all constructors of Cell initialise _occupied and _pieceOccupying, which means that these can have garbage values in some Cell objects.

  2. Furthermore, your makeMove dereferences getOccupyingPiece() unconditionally - it doesn't check for null.

  3. Finally, you only have pawns on your board, which means (0, 0) does not contain a piece - and because of 1., the Cell object contains a garbage value in its _pieceOccupying, so you access random memory and crash.

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • Hi, thanks for your answer! The memory error seems to happen even when I have all the pieces on the board, and also when I call the method with the correct pawn coordinates. (I just didn't want to put the whole board initialization in because it is similar to how pawns are made) – user475680 Jan 08 '15 at 12:51
  • @user475680 I did my best with the info you gave us. If that doesn't help, please provide more complete info - an [MCVE](http://stackoverflow.com/help/mcve). That is **minimal** code which I could directly copy & paste into e.g. [ideone](http://ideone.com/) and observe the same behaviour as you. – Angew is no longer proud of SO Jan 08 '15 at 13:17
  • Thanks for your help. It seems to work OK now, oddly enough. I may have incorrectly de-referenced it in debugging or something. I didn't change anything apart from assigning nullptr at the start and checking for null values as you suggested. I'm just not that comfortable yet with C++ memory allocation so I figured that maybe I was doing something wrong or problematic – user475680 Jan 08 '15 at 14:15