Class string variable loses its value when I put that class inside a map in another class

Question

Well the problem is, that my room classes loose their string variable "name" values when I put those classes inside a map in TransparentMaze class. I can't understand where they go.. Is the problem related with the map comparator or something else, hopefully somebody can help.

#include <iostream>
#include <string>
#include <list>
#include <map>

using namespace std;

class Point{
    int x;
    int y;
    public:
        Point(int, int);
        ~Point(){};
        int getX() ;
        int getY() ;
};

struct cmp_str {
    bool operator()(Point p1, Point p2) {
        if (p1.getY() < p2.getY() || p1.getY() == p2.getY() && p1.getX() < p2.getX()) {
            return true;
        }
        return false;
    }
};

class Room{
    Room *north;
    Room *south;
    Room *east;
    Room *west;
    public:
        int createDirection[4];
        std::string name;
        Room(std::string);
        ~Room();
        std::string getName();
        std::list<std::string> findPathTo(std::string);
        void fillMap(map<Point, Room*, cmp_str>*, int*, int*);
        Room& createNorth(std::string);
        Room& createSouth(std::string);
        Room& createEast(std::string);
        Room& createWest(std::string);
        Room& getNorth();
        Room& getSouth();
        Room& getEast();
        Room& getWest();
        Room& get(std::string);
        Room& get(std::list<std::string>);
        void setNorth(Room*);
        void setSouth(Room*);
        void setEast(Room*);
        void setWest(Room*);
        //bool validatePath(std::list<std::string>);
};

class Maze{
    Room *entry;
    public:
        Maze(string);
        ~Maze();
        Room& getEntry();
};

class TransparentMaze{
    std::map<Point, Room*, cmp_str> maze;
    int width;
    int height;
    public:
        TransparentMaze(Maze);
        ~TransparentMaze(){};
        std::map<Point, Room*, cmp_str>& getMaze();
};

TransparentMaze::TransparentMaze(Maze maze) {
    maze.getEntry().fillMap(&this->maze, &this->width, &this->height);
}

std::map<Point, Room*, cmp_str>& TransparentMaze::getMaze() {
    return maze;
}

Point::Point(int x, int y) {
    this->x = x;
    this->y = y;
}

int Point::getX() {
    return x;
}

int Point::getY() {
    return y;
}

Maze::Maze(std::string name) {
    entry = new Room(name);
}

Maze::~Maze() {
    delete entry;
}

Room& Maze::getEntry() {
    return *entry;
}

Room::Room(std::string name) {
    this->name = name;
    for (int i = 0; i < sizeof(createDirection) / sizeof(int); i++) {
        createDirection[i] = 0;
    }
    north = NULL;
    south = NULL;
    east = NULL;
    west = NULL;
}

Room::~Room() {
    for (int i = 0; i < sizeof(createDirection) / sizeof(int); i++) {
        switch(i) {
            case 0:
                if (createDirection[i] == 1) {
                    delete north;
                }
                break;
            case 1:
                if (createDirection[i] == 1) {
                    delete south;
                }
                break;
            case 2:
                if (createDirection[i] == 1) {
                    delete east;
                }
                break;
            case 3:
                if (createDirection[i] == 1) {
                    delete west;
                }
                break;
        }
    }
}

Room& Room::get(std::string direction) {
    switch(direction[0]) {  
        case 'N': case 'n': return (this->north == NULL ? *this : *this->north);
        case 'S': case 's': return (this->south == NULL ? *this : *this->south);
        case 'E': case 'e': return (this->east == NULL ? *this : *this->east);
        case 'W': case 'w': return (this->west == NULL ? *this : *this->west);
        default: break;
    }
    return *this;
}

Room& Room::get(std::list<std::string> pathList) {
    std::list<std::string>::iterator it;
    Room *room = this;

    for (it = pathList.begin(); it != pathList.end(); it++) {
        if (&(*room) == &room->get(*it)) {
            return *this;
        }
        room = &room->get(*it);
    }
    return *room;
}

void Room::fillMap(map<Point, Room*, cmp_str> *maze, int *width, int *height) {
    std::list<std::list<std::string>> listOfLists;
    std::list<std::list<std::string>>::iterator it1;
    Room *r;
    int n, counter;

    *width = *height = 0;
    listOfLists.push_back(std::list<std::string>());
    for (;;) {

        counter = 0;
        for (it1 = listOfLists.begin(); it1 != listOfLists.end(); it1++) {
            r = &this->get(*it1);

            //Point p((int)r->getName()[0] - 64, (int)r->getName()[1] - 48);
            //cout << r->getName() << " " << &*r << "  " << &this->get(*it1) << endl;
            maze->insert(std::make_pair(Point((int)r->getName()[0] - 64, (int)r->getName()[1] - 48), &*r));
            //cout << r->getName() << endl;
            std::string lastRoom;
            n = 0;

            if (it1->size() > 0) {
                lastRoom = *--it1->end();
            }
            if (&r->getNorth() != NULL && lastRoom.compare("S") != 0) {
                n++;
                counter++;
                if (n == 1) {
                    it1->push_back("N");
                }  else {
                    std::list<std::string> bufferList(it1->begin(), --it1->end());
                    bufferList.push_back("N");
                    listOfLists.push_back(bufferList);
                }
            }
            if (&r->getEast() != NULL && lastRoom.compare("W") != 0) {
                n++;
                counter++;
                if (n == 1) {
                    it1->push_back("E");
                }  else {
                    std::list<std::string> bufferList(it1->begin(), --it1->end());
                    bufferList.push_back("E");
                    listOfLists.push_back(bufferList);
                }
            }
            if (&r->getWest() != NULL && lastRoom.compare("E") != 0) {
                n++;
                counter++;
                if (n == 1) {
                    it1->push_back("W");
                }  else {
                    std::list<std::string> bufferList(it1->begin(), --it1->end());
                    bufferList.push_back("W");
                    listOfLists.push_back(bufferList);
                }
            }
            if (&r->getSouth() != NULL && lastRoom.compare("N") != 0) {
                n++;
                counter++;
                if (n == 1) {
                    it1->push_back("S");
                }  else {
                    std::list<std::string> bufferList(it1->begin(), --it1->end());
                    bufferList.push_back("S");
                    listOfLists.push_back(bufferList);
                }
            }
        }

        if(counter == 0) {
            /*cout << listOfLists.size() << endl;
            map<Point, Room*, cmp_str>::iterator it;
            for (it = maze->begin(); it != maze->end(); it++) {
                cout << "X: "<< it->first.getX() << " Y: " << it->first.getY() << "; " << (*it->second).getName() << endl;
            }*/
            return;
        }
    }
}

std::list<std::string> Room::findPathTo(std::string roomName) {
    std::list<std::list<std::string>> listOfLists;
    std::list<std::list<std::string>>::iterator it1;
    Room *r;
    int n, counter;

    listOfLists.push_back(std::list<std::string>());
    for (;;) {

        counter = 0;
        for (it1 = listOfLists.begin(); it1 != listOfLists.end(); it1++) {
            r = &this->get(*it1);
            std::string lastRoom;
            n = 0;

            if (it1->size() > 0) {
                lastRoom = *--it1->end();
            }
            if (r->getName().compare(roomName) == 0) {

                return *it1;
            }
            if (&r->getNorth() != NULL && lastRoom.compare("S") != 0) {
                n++;
                counter++;
                if (n == 1) {
                    it1->push_back("N");
                }  else {
                    std::list<std::string> bufferList(it1->begin(), --it1->end());
                    bufferList.push_back("N");
                    listOfLists.push_back(bufferList);
                }
            }
            if (&r->getEast() != NULL && lastRoom.compare("W") != 0) {
                n++;
                counter++;
                if (n == 1) {
                    it1->push_back("E");
                }  else {
                    std::list<std::string> bufferList(it1->begin(), --it1->end());
                    bufferList.push_back("E");
                    listOfLists.push_back(bufferList);
                }
            }
            if (&r->getWest() != NULL && lastRoom.compare("E") != 0) {
                n++;
                counter++;
                if (n == 1) {
                    it1->push_back("W");
                }  else {
                    std::list<std::string> bufferList(it1->begin(), --it1->end());
                    bufferList.push_back("W");
                    listOfLists.push_back(bufferList);
                }
            }
            if (&r->getSouth() != NULL && lastRoom.compare("N") != 0) {
                n++;
                counter++;
                if (n == 1) {
                    it1->push_back("S");
                }  else {
                    std::list<std::string> bufferList(it1->begin(), --it1->end());
                    bufferList.push_back("S");
                    listOfLists.push_back(bufferList);
                }
            }
        }

        if(counter == 0) {
            return std::list<std::string>();
        }
    }
}

/*bool Room::validatePath(std::list<std::string> pathList) {
    std::list<std::string>::iterator it;
    Room *room = this;

    for (it = pathList.begin(); it != pathList.end(); it++) {
        if (&(*room) == &room->get(*it)) {
            return false;
        }
        room = &room->get(*it);
    }
    return true;
}*/

std::string Room::getName(){
    return name;
}

Room& Room::createNorth(std::string name) {
    north = new Room(name);
    createDirection[0] = 1;
    north->setSouth(this);
    return *north;
}

Room& Room::createSouth(std::string name) {
    south = new Room(name);
    createDirection[1] = 1;
    south->setNorth(this);
    return *south;
}

Room& Room::createEast(std::string name) {
    east = new Room(name);
    createDirection[2] = 1;
    east->setWest(this);
    return *east;
}

Room& Room::createWest(std::string name) {
    west = new Room(name);
    createDirection[3] = 1;
    west->setEast(this);
    return *west;
}

Room& Room::getNorth() {
    return *north;
}

Room& Room::getSouth() {
    return *south;
}

Room& Room::getEast() {
    return *east;
}

Room& Room::getWest() {
    return *west;
}

void Room::setNorth(Room *room) {
    north = room;
}

void Room::setSouth(Room *room) {
    south = room;
}

void Room::setEast(Room *room) {
    east = room;
}

void Room::setWest(Room *room) {
    west = room;
}

int main() {
    Maze maze("A5");

    maze.getEntry().createSouth("A6").createEast("B6").createSouth("B7").createWest("A7")
        .getEast().createEast("C7").createNorth("C6").createEast("D6").createEast("E6")
        .getWest().createNorth("D5").createEast("E5").getWest().getSouth().getWest()
        .createNorth("C5").createWest("B5").createNorth("B4").createWest("A4").createNorth("A3")
        .getSouth().getEast().getSouth().getEast().createNorth("C4").createNorth("C3")
        .createWest("B3").getEast().createNorth("C2").createWest("B2").createWest("A2")
        .createNorth("A1").getSouth().getEast().getEast().createEast("D2").createSouth("D3")
        .createSouth("D4").createEast("E4").createNorth("E3").createNorth("E2").createNorth("E1")
        .createWest("D1").createWest("C1").createWest("B1").getEast().getEast().getEast()
        .createEast("F1").createEast("G1").createEast("H1").createSouth("H2").createEast("I2")
        .createSouth("I3").createEast("J3").createNorth("J2").createNorth("J1").createWest("I1")
        .getEast().getSouth().getSouth().getWest().getNorth().getWest().getNorth().getWest()
        .getWest().getWest().getSouth().getSouth().createEast("F3").createEast("G3")
        .createNorth("G2").createWest("F2").getEast().getSouth().createEast("H3").createSouth("H4")
        .createEast("I4").createEast("J4").createSouth("J5").createWest("I5").createWest("H5")
        .getEast().createSouth("I6").createEast("J6").createSouth("J7").createWest("I7")
        .createWest("H7").createWest("G7").getEast().createNorth("H6").createWest("G6")
        .createWest("F6").createNorth("F5").createNorth("F4").createEast("G4").createSouth("G5")
        .getNorth().getWest().getSouth().getSouth().createSouth("F7").createWest("E7")
        .createWest("D7");

    //cout << (int)'A' << endl;
    //cout << (int)'1' << endl;

    /*std::list<std::string> list(maze.getEntry().findPathTo("E7"));
    std::list<std::string>::iterator it;

    for (it = list.begin(); it != list.end(); it++) {
        cout << *it << endl;
    }*/

    //cout << sizeof(maze.getEntry()) << endl;



    //Here it has still a value..
    cout << maze.getEntry().getName().length() << endl;
    TransparentMaze tMaze(maze);
    //Here not anymore.. why it's gone?
    cout << maze.getEntry().getName().length() << endl;




    //tMaze.getMaze();
    /*map<Point, Room*, cmp_str>::iterator it;
    for (it = tMaze.getMaze().begin(); it != tMaze.getMaze().end(); it++) {
        cout << "X: "<< it->first.getX() << " Y: " << it->first.getY() << "; " << it->second->name << endl;
    }*/


}

Please read this and edit your post accordingly: [SSCCE](http://sscce.org/) — ildjarn, Oct 20 '11 at 18:16
It's a lot of code, but it does ask a specific question (near the end), and provides a concise compilable example. — Chad, Oct 20 '11 at 18:24

score 1 · Accepted Answer · answered Oct 20 '11 at 19:16

This is a bit complex, but here is what is happening. In short the TransparentMaze constructor gets a Maze instance passed by value, but the Maze class is not safe to copy (it contains a pointer). The most straightforward way to fix this is by making Maze non-copyable (more below) and making TransparentMaze take a reference to Maze (Maze&) instead of a copy.

class Maze{
    Room *entry;

    // these two lines make it non-copyable
    Maze(const Maze&);            // <-- private copy constructor
    Maze& operator=(const Maze&); // <-- private assignment operator

    public:
        Maze(string);
        ~Maze();
        Room& getEntry();
};


class TransparentMaze{
    std::map<Point, Room*, cmp_str> maze;
    int width;
    int height;
    public:
        TransparentMaze(Maze&);  // pass by reference
        ~TransparentMaze(){};
        std::map<Point, Room*, cmp_str>& getMaze();
};

TransparentMaze::TransparentMaze(Maze& maze) {
    maze.getEntry().fillMap(&this->maze, &this->width, &this->height);
}

When TransparentMaze is constructed, the constructor gets a copy of Maze which makes a copy of the entry pointer. This copy is deleted when the TransparentMaze constructor finishes and the ~Maze destructor deletes the entry pointer. When you try to used that deleted entry pointer later (in the original maze instance it was copied from) you will get undefined behavior (I got a crash on my system).

score 0 · Answer 2 · edited May 23 '17 at 12:03

That's a lot of code. There are a lot of issues with it, but the actual problem you have is this -- your objects perform shallow copies, and cleanup themselves upon destruction. Shallow copies used in this manner will not work properly, as memory pointed to in subsequent copies of the objects will be freed when one copy is destroyed -- leaving dangling pointers in the remaining objects.

// This function takes a COPY of the Maze object
TransparentMaze::TransparentMaze(Maze maze)

// But the Maze object cannot be safely copied
// because it only does a shallow copy
// and destroy's its entry upon destruction
Maze::~Maze() {
   delete entry;
}

Consider moving your raw pointers in Maze and Room to smart pointer types, or you will have to write copy-constructors for all of your classes.

See this question for a discussion of deep-copy vs. shallow-copy.

Thanks a lot, now I see my mistake - have to pass Maze class reference inside the TransparentMaze class. Works now. — Jaak, Oct 20 '11 at 18:49

Class string variable loses its value when I put that class inside a map in another class

2 Answers2