Getting constant empty line output when retrieving a string from a pointer

Question

This is making me think I'm not using the pointer correctly.

main.cpp

#include <iostream>
#include "room.h"

void initializeRooms(std::vector<Room*>& rooms);

int main() {
  std::vector<Room*> rooms;
  initializeRooms(rooms);
  Room* r = rooms[0];
  std::cout << "You are in room " << r->getName() << "\n";
  return 0;
}

void initializeRooms(std::vector<Room*>& rooms) {
  Room roomOne {"roomOne"};
  Room roomTwo {"roomTwo"};
  Exit exit { &roomOne, &roomTwo };
  roomOne.addExit(&exit);
  rooms.push_back(&roomOne);
  rooms.push_back(&roomTwo);
}

exit.cpp

class Room;

struct Exit {
  Room* room_one;
  Room* room_two;
};

room.h

#ifndef ROOM_H
#define ROOM_H

#include <string>
#include <vector>
#include "exit.cpp"

class Room {
private:
  std::string name;
  std::vector<Exit*> exits;

public:
  Room(std::string n): name{n} {

  }

  void addExit(Exit* e);
  std::string& getName();
};

#endif

room.cpp

#include "room.h"

void Room::addExit(Exit* e) {
  exits.push_back(e);
}

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

So in the main file, when the cout is called, I just see a constant loop of empty lines being output when i run the compiled file. Just keeping it simple now and using a makefile with clang

all: build

build: main.o exit.o room.o
    clang++ -std=c++11 main.o exit.o room.o -o simplegame

main.o: main.cpp
    clang++ -std=c++11 -c main.cpp

exit.o: exit.cpp
    clang++ -std=c++11 -c exit.cpp

room.o: room.cpp
    clang++ -std=c++11 -c room.cpp

clean:
    rm -rf *o simplegame

`roomOne`, `roomTwo` and `exit` are local to `initializeRooms`. You are pushing pointers to the first two into a vector that lives beyond the scope of the function, leaving it with dangling pointers and leading to undefined behaviour. — juanchopanza, Sep 25 '13 at 19:38
@Leeor That syntax is fine I believe for C++11 (and in some cases preferred). See http://stackoverflow.com/a/18222927/1843316 — Sam Cristall, Sep 25 '13 at 19:42
Ok, thanks. Please retag, i was assuming standard c++ (didn't notice the makefile args) — Leeor, Sep 25 '13 at 19:43
I'm not baring anything with you. Nothing personal, but you're not my type. — R. Martinho Fernandes, Sep 25 '13 at 21:23

score 2 · Accepted Answer · answered Sep 25 '13 at 19:43

Putting aside that you should not use raw pointers unless you fully understand what you are doing, following code have the problem:

void initializeRooms(std::vector<Room*>& rooms) {
  Room roomOne {"roomOne"};    // this is local object
  Room roomTwo {"roomTwo"};    // this is local object as well 
  Exit exit { &roomOne, &roomTwo }; // exit is also local object and now it holds pointers to other 2 local objects
  roomOne.addExit(&exit);  // same stuff as before
  rooms.push_back(&roomOne); // now you are inserting pointer to local object into vector
  rooms.push_back(&roomTwo); // doing that again
} // now all local objects to this function automatically destroyed and you hold pointers to garbage in rooms

So instead of using local objects you need to create them on the heap. As I said before I would also recommend to use shared_ptr and weak_ptr (later is neccessary because you have cyclic reference).

Good to know thank you. Just been going through the 4th edition of The C++ Programming language, been trying to implement some basics to better have it sunk in. Haven't really gotten to much on the shared_ptr, unique_ptr and weak_ptr yet. — agmcleod, Sep 25 '13 at 23:25
@agmcleod: There is completely no need for you to use raw pointers in this demo. `std::vector` holds value, so you can just `push_back` the `Room` created into it. Or better `emplace_back`. — Siyuan Ren, Sep 26 '13 at 00:20

Getting constant empty line output when retrieving a string from a pointer

1 Answers1