The class I have created seems to be incorrectly setting it's local variables

Question

Sorry for my inexperience in the matter and if its a simple answer, but I am writing a C++ program for an assignment and cannot for the life of me find a solution.

I have created a class called Door that possesses a local variable that stores the room it's connected to. Additionally, each door is connected to one another through another local variable. The local variable that records the door connected to each door is a pointer (a requirement of the assignment).

The door class passes back the door its connected to through dereferencing the pointer (through the * operator).

Another class, Room, stores four doors in variables names after the four directions (North, East, etc) and is identified by a simple integer that can be passed through a function to receive a shared pointer that points to the room object.

The problem I am having is in my class that is responsible for creating the rooms and populating them with doors, then connecting the doors to one another, the program is able to set the local variables of the Door class, however, once the door object that's variables were just set is referred back to it appears to have incorrectly set variables.

In the example below the integers of the rooms that are being created are 1 and 2.

This is not the full code, just a snippet of relevant info

class Door{

    private:
        Door *_connectedDoor;
        int _connectedRoom;
        bool _isEntrance;
        bool _isExit;
};

void Door::connectDoor(Door connectedDoor){

    if( _isExit == true || _isEntrance == true)
    {
        std::cout << "Cannot connect, is an entrance / exit";
    }

    else
    {   
        _connectedDoor = &connectedDoor;
    }
};

Door Door::getConnectedDoor(){

    return *_connectedDoor;
}

int Door::getRoom(){
   std::cout << std::endl << "*** passing connected room:" << _connectedRoom << " ***" << std::endl;
   return _connectedRoom;
};

void Door::setRoom(int room){

    std::cout << std::endl << "*** setting connected room:" << room << " ***" << std::endl;
    _connectedRoom = room;

};





class Room{
  private:
     int _id;
     std::string _description;
     Door _northDoor;
};

void Room::setNorth(Door north){
_isDoorNorth = true;
    _northDoor = north;
    std::cout << std::endl << "*** configuring doors room internally to:" << _id << " ***" << std::endl;
    north.setRoom(_id);
};

class Game

void Game::connectRooms(int room1, int room2, std::string startingDirection)
{
    std::cout << std::endl << "*** connecting rooms ***" << std::endl;

    std::shared_ptr<dungeon::Room> startingRoom = _currentDungeon.retrieveRoom(room1);
    std::shared_ptr<dungeon::Room> endingRoom = _currentDungeon.retrieveRoom(room2);
    dungeon::Door startingRoomDoor = *new dungeon::Door();
    dungeon::Door endingRoomDoor = *new dungeon::Door();

    std::cout << std::endl << "*** configuring room:" << startingRoom->id() << " ***" << std::endl;
    std::cout << std::endl << "*** configuring room:" << endingRoom->id() << " ***" << std::endl;

    startingRoom->setNorth(startingRoomDoor);
    endingRoom->setSouth(endingRoomDoor);

    std::cout << std::endl << "*** configuring doors room:" << startingRoomDoor.getRoom() << " ***" << std::endl;
    std::cout << std::endl << "*** configuring doors room:" << endingRoomDoor.getRoom()  << " ***" << std::endl;

    startingRoomDoor.connectDoor(endingRoomDoor);
    endingRoomDoor.connectDoor(startingRoomDoor);
};

Output:

*** connecting rooms ***

*** configuring room:1 ***

*** configuring room:2 ***

*** configuring doors room internally to:1 ***

*** setting connected room:1 ***

*** configuring doors room internally to:2 ***

*** setting connected room:2 ***

*** configuring doors room:
*** passing connected room:1835344 ***
1835344 ***

*** configuring doors room:
*** passing connected room:1835344 ***
1835344 ***

Expected:

*** configuring doors room:
*** passing connected room:1 ***
1 ***

*** configuring doors room:
*** passing connected room:2 ***
2 ***

Do you come from Java/C# environment? You seem to treat objects like references and pointers usage is invalid. We can try to explain many of the errors here, but you may want to get [a good C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) to learn from. — Yksisarvinen, Oct 05 '19 at 11:18
You should really edit your question to provide a [mcve]. Having said that, in `Door::connectDoor` you have `_connectedDoor = &connectedDoor;` -- that's taking the address of the local variable `connectedDoor` and will result in undefined behaviour. — G.M., Oct 05 '19 at 11:18
The real problem here is that you use `references` and `pointers` without knowing how. — DimChtz, Oct 05 '19 at 11:20
I would assume it’s because copies are made of the objects you pass around so they aren’t what you think they are. Maybe you want to use references instead? — Sami Kuhmonen, Oct 05 '19 at 11:20
Yeah, I come from a very very heavy Java background. I'll try do more research into pointers and see if I can sort it out, thank you for the very very swift replies. — Harry NONEOFYOBus, Oct 05 '19 at 11:22
When learning C++, it's best to forget everything you learned about Java and start from scratch :) For example, `void Room::setNorth(Door north)` will copy `Door`. `north` object will have nothing in common with the object that was passed as the argument (except that it will have exactly the same values inside). — Yksisarvinen, Oct 05 '19 at 11:35
Side note: In C++, you usually compare booleans with `if(condition)` — Aconcagua, Oct 05 '19 at 11:55

score 3 · Accepted Answer · answered Oct 05 '19 at 11:36

First of all: Nice to see, that you use std::shared_ptr! I think your problem is the copying of objects. Since I don't know if it's just a typo or something, I will explain it a bit more detailed. Let's take a look at some of your code:

dungeon::Door startingRoomDoor = *new dungeon::Door(); What this will do is the following:

Create a new instance of dungeon::Door using the new-operator. this returns a pointer to the new instance.
Dereference this pointer with *.
Initialize startingRoomDoor with the new instance, thereby calling a copy-constructor and copying the new instance.
The pointer to the new instance goes out of scope and you get a memory leak.

You can fix this using a std::shared_ptr like you did with startingRoom and endingRoom. Alternatively, you can just write dungeon::Door startingRoomDoor; and have an initialized instance of a dungeon::Door. In this case the constructor is called automatically.

Next will be your Room-functions:

void Room::setNorth(Door north){
_isDoorNorth = true;
    _northDoor = north;
    std::cout << std::endl << "*** configuring doors room internally to:" << _id << " ***" << std::endl;
    north.setRoom(_id);
}

Here you pass north by value. Which means, that the value you pass to this function actually gets copied and inside the function you only modify a copy of a Room. At the end of the function north gets destroyed, since it's scope ends there. You can fix this, if you pass by reference like this: void Room::setNorth(Door &north) Now you actually modify the value you passed to the function instead of a copy.

But be aware! The line _northDoor = north; will still copy north. Even if north is passed by reference. Maybe you want to store just a pointer instead of a whole instance? You could also use std::shared_ptr for this.

Can I please have an option to give you more Karma, thank you so much for the detailed reply and help. <3 <3 <3 — Harry NONEOFYOBus, Oct 05 '19 at 11:44

The class I have created seems to be incorrectly setting it's local variables

This is not the full code, just a snippet of relevant info

1 Answers1