Some problems with an abstract class

This is a discussion on Some problems with an abstract class within the C++ Programming forums, part of the General Programming Boards category; Hey guys, I am having a few problems with this abstract class I am working on.
We are building a ...

Some problems with an abstract class

Hey guys, I am having a few problems with this abstract class I am working on.

We are building a chess game in class, and using inheritance for the different pieces.

So my base class is simply called PIECE, and there are six classes that inherit from it: PAWN, ROOK, KNIGHT, BISHOP, QUEEN, and KING.

Well, most of the functionality of all the pieces is the exact same, so most of the functionality lies in the PIECE class itself. However, I have made the destructor virtual, as well as a function called GetPossibleMoves and also WriteToXML.

GetPossibleMoves is virtual obviously because every type of piece has a different set of possible moves. WriteToXML is virtual because...well, for similar reasons.

Anyways, the problem is this:

For the most part in my program I refer to all the pieces as PIECE in general. Very very rarely do I actually refer to them by their child class names. My desire is that when I call the GetPossibleMoves function from the base class of PIECE, that it automatically calls the correct child version of it. For all intents and purposes it should be doing so...however...it's not...

I orginally had GetPossibleMoves defined as basically an empty function on the PIECE level, and then on its childrens' levels, it was defined for each piece to get the moves of that piece.

I noticed that whenever I called GetPossibleMoves, it was returning to me a set of 0's...which means that no moves are possible....well...the only way that is possible is if it is calling the parent GetPossibleMoves, and not the child version.

So I decided to change the parent function from virtually an empty function to this:

virtual bitset <64> GetPossibleMoves ( ) = 0;

This brings up a different problem: It makes it a pure abstract class. Pure abstract classes cannot be instantiated or worked with, except with pointers. Well, right now my program isn't using pointers, and it would take a great deal of work to make it use pointers.....although it is one option.

For reference, I will post the code of where the pieces are being created, and then where they are being used.

You have to use pointers or references if you want the virtual mechanism to do its job.

>> newPiece = ROOK();
What you are doing here and with the other types is called slicing. You are copying a derived piece to a base class piece which only saves the base class information. To use polymorphism, newPiece needs to be a PIECE* or PIECE&.

For a chess game, you know exactly how many pieces there will be on the board, so you have some options on how to create them. The normal method of creating objects for use with polymorphism is to use new to dynamically allocate the memory for the piece and save that pointer somewhere (e.g. in your chessPieces vector/array). When the board is destroyed, you should loop through and delete all the pieces. You can also switch to a boost::ptr_vector<PIECE> or vector<shared_ptr<PIECE> > which will do the cleanup automatically. Another option is to include statically creating the pieces (since you know exactly how many there will be) and storing pointers without having to remember to delete them.

Regardless, you will have to be doing all of your function calling via pointers or references. I would change all functions that take a PIECE and make them take a const PIECE& instead (assuming your code is const correct, otherwise just use a PIECE&). All code that accesses a PIECE directly from the chessPieces container would treat it as a pointer, otherwise you would use references. This will minimize the code that needs to change, as references use the same syntax as regular objects.

I would change GetPiece to return a reference or const reference, so your code that uses it would look like this:

Code:

PIECE& pieceSelected = gameBoard.GetPiece ( c, r );

This way, you won't be copying and slicing, and the virtual function mechanism will still work. If GetPiece can return an invalid piece, you might have to use a pointer there instead.

I am editing this post a bit based on some new stuff that has come up. I discovered that a part of my problem was in the copy constructor of my BOARD class. I was passing the BOARD around to various parts of the program, by value, and therefore the copy constructor and the destructor were being called often. Well, whenever the destructor was being called, it was killing my piece information. I was hesitant to create new copies of the pieces every time I wanted to pass the BOARD around, so I decided to just pass the game board by reference using pointers and stuff, so that way the destructor wouldnt get called until the very end of the game when I want it to be called. That way there is no bad stuff happening to my piece data. So that problem is solved. I clicked on the pieces and they began to work for the most part, EXCEPT for bishops. An odd problem, that only bishops wouldn't work?

[/END 2ND EDIT]

The pieces are stored in a map like thus:

Code:

map < int, PIECE * > chessPieces;

The segmentation fault is happening in the following function. It is the same function as it was happening before, and in fact it is happening on the same line of code. The difference is that the fault is isolated to only bishops now. That code follows:

Those are the pieces of code that are probably the most prone to have pointer errors in them. I am searching for where the error could be, but so far I haven't found it yet, so you guys see anything blatantly obvious or annoyingly subtle that I might be missing?

I thought it might be useful to post this function as well. It is a short but important one:

Since you really don't want to be copying the BOARD around, you should make the copy constructor and copy assignment operator private (you don't have to implement them, just declare them in the private section of the class). That way, if you ever accidentally make a typo and copy the BOARD, the compiler will flag the error immediately and you can fix it instead of having it cause a crash later do to the deletion of the pieces.

You also should make PIECE::GetPossibleMoves pure virtual. That way, your compiler will prevent you from instantiating PIECEs that aren't actually derived classes.

Rule: Make as many possible error situations as possible fail at compilation. The compiler is your best friend when it comes to ensure program correctness, so work with it, not against it. Use non-copyable objects and let the compiler ensure you don't copy them. Use abstract classes and let the compiler ensure you don't instantiate them. Use smart pointers and let the compiler ensure the memory is freed.