I've got three classes: Room, Door and World
#include <set>;
using namespace std;
class Door; // Forward declaration
class Room {
public:
Door* door1;
Door* door2;
Room(){}
~Room() {
delete door1;
door1 = 0;
delete door2;
door2 = 0;
}
};
class Door {
public:
Room* roomA;
Room* roomB;
Door(Room* roomA, Room* roomB) {
this->roomA = roomA;
this->roomB = roomB;
linkRooms(); // This sets up the Door-Pointers in the Rooms
// so they know about the new door.
}
~Door() {
// Returns the room-owned pointer pointing at this door
getMyRoomPointer(roomA) = 0;
getMyRoomPointer(roomB) = 0;
}
Door * & getMyRoomPointer(Room * const & room) {
if (room->door1 == this) return room->door1;
else return room->door2;
}
void linkRooms() {
roomA->door1 = this;
roomB->door2 = this;
}
};
class World {
public:
std::set<Room*> rooms;
World() {
// Set up two rooms and link them using a door
Room* newRoom = new Room();
rooms.insert(newRoom);
Room* anotherNewRoom = new Room();
rooms.insert(anotherNewRoom);
new Door(newRoom, anotherNewRoom);
}
~World() {
// Iterate over the rooms and call delete on all of them
for (std::set<Room*>::iterator it = rooms.begin(); it != rooms.end(); ++it) {
delete *it;
}
}
};
int main() {
World world;
return 0;
}
When running main, the constructor fills the world with just two rooms and a door as a link between them. After main returns, world should be deleted and all the rooms and doors also should be taken care of.
The thing is, that my Door destructor is never called. So the Door pointers inside the rooms are not set to null and I get an error when the room "at the other side" tries to delete the same door.
When I just create an instance of Door, and than delete it right afterwards, I'm not facing any problems:
int main(){
Room oneRoom;
Room anotherRoom;
Door* door = new Door(&oneRoom, &anotherRoom);
delete door; // works just fine
return 0;
}
Questions: Why is the Door constructor not called? Is setting up Door like in the first example allowed?
I know, that I'm double deleting my room's Door pointers, and that I could (and should) use SmartPointers. Right now I'm just wondering why I am facing that kind of behavior. I'm still new to C++, after all.
I did set up a runnable example now, that reproduces the error.
You call delete
before Door
has been defined. Therefore the behaviour of the program is undefined, and the destructor is not guaranteed to be called.
A quote from the standard (draft) [expr.delete]:
- If the object being deleted has incomplete class type at the point of deletion and the complete class has a non-trivial destructor or a deallocation function, the behavior is undefined.
Solution: If the destructor of the type is non-trivial (i.e. user defined, such as the ~Door
, then never delete such object until the type is complete). In this case, define Door
, before the functions that call delete.
As a general rule, you can never call a member function unless the class type is complete. Unfortunately in the case of destructor, it may not always be possible for the compiler to catch the bug. PS. g++ does warn about your program: warning: possible problem detected in invocation of delete operator: [-Wdelete-incomplete]