Problem with std::vector and/or polymorphism

Question

I looked everywhere for an answer but can't find one. Below is my short program in visual studio 2017. It does not print neither of the "rooms" I added - it posts random values. The code compiles perfectly. just the output is wrong. Most the code is irrelevant I think, I just honestly don't know WHAT I did wrong. I think its probably in the vector/inheritance parts.

EDIT: SOLVED quickly. I inserted a local variable's pointer to my vector. thanks.

Room.h :

#pragma once
#include <vector>

class Room {
protected:
    int i;
    int j;
    int size;
    char** objs;

public:
    Room(int _i = 0, int _j = 0, int _size = 0);
    int get_i() const { return i; }
    int get_j() const { return j; }
    int get_size() const { return size; }
    char** get_objs() const { return objs; }
    virtual void make_room() = 0;
};

class Maze : public Room {
public:
    Maze(int _i, int _j, int _size);
    void make_room();
};

class Board {
    std::vector<Room*> rooms;

public:
    Board() {}
    void add_room(int i, int j, int v);
    void print_a_room() const;
};

Room.cpp:

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

Room::Room(int _i, int _j, int _size) : i(_i), j(_j), size(_size) {
    objs = new char*[_size];
    for (int i = 0; i < _size; i++) {
        objs[i] = new char[_size];
        for (int j = 0; j < _size; j++) {
            objs[i][j] = ' ';
        }
    }
}

Maze::Maze(int _i, int _j, int _size) : Room(_i, _j, _size) {
    for (int i = 0; i < size; i++) {
        for (int j = 0; j < size; j++) {
            objs[i][j] = '*';
        }
    }
    make_room();
}

void Maze::make_room() {
    int x = 1;
}

void Board::add_room(int i, int j, int v) {
    rooms.push_back(&Maze(i, j, v));
}

void Board::print_a_room() const {
    Room* r = rooms.back();
    std::cout << r->get_i() << "," << r->get_j() << std::endl;
}

main.cpp:

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

int main() {
    Board b;
    b.add_room(1, 2, 10);
    std::cout << "hello" << std::endl;
    b.add_room(3, 4, 5);
    b.print_a_room();
    return 0;
}

Help will be greatly appreciated.

you're right. its a local variable. I didn't even consider that .... stupid mistake. thanks. — Its_a_me, Jan 22 '19 at 11:13
@Itai Instead of editing the question and saying that you've solved the problem, accept one of the answers below that gave you the solution. — Ted Lyngmo, Jan 22 '19 at 11:16
I will look for guides on upping the warning levels, yea. thanks. — Its_a_me, Jan 22 '19 at 11:25

jrok · Accepted Answer · 2019-01-22T11:56:10.310

1

This shouldn't compile in the first place (here's why), but since your compiler apparently allows it...

You are storing adresses of temporary objects:

void Board::add_room(int i, int j, int v) {
    rooms.push_back(&Maze(i, j, v));   // <---- There
}

The Maze objects created in this expression die immediately after the expression ends and you're left with dangling pointers, pointing nowhere in particular.

It's usualy not needed to store pointers in a vector unless you want polymorphic behaviour. In that case, use a suitable smart pointer, such as shared_ptr or unique_ptr.

edited Jan 22 '19 at 11:56

answered Jan 22 '19 at 11:07

jrok

54,456
9
109
141

It's ok to store pointers to rooms. just pass a pointer to dynamically allocated object (via new). If the vector will be storing Rooms by value the [object slicing](https://www.geeksforgeeks.org/object-slicing-in-c/) will happen. – Zak Jan 22 '19 at 11:13
@Zak Right, haven't thought abou their class structure. Will edit. – jrok Jan 22 '19 at 11:14
thanks! for some reason I forgot about the whole local-variable scope thing. will try to remember from now on. – Its_a_me Jan 22 '19 at 11:18
@rustyx Because it's taking the address of an rvalue. – jrok Jan 22 '19 at 11:54

score 1 · Answer 2 · edited Jan 22 '19 at 11:21

1

The problem is this: rooms.push_back(&Maze(i, j, v))

The problem is that the current object is allocated on the stack, and after this line, this object would be destroyed.

Changing from Room* to Room is NOT a solution - you cannot have polymorphism if you do this.

Best solution: rooms.push_back(new Maze(i, j, v)).

NOTE: You need to define the Rule of Three, because there is shared memory. When you do a pop_back, first call operator delete on current element of room, then call pop_back method of std::vector.

edited Jan 22 '19 at 11:21

P.W

26,289
6
39
76

answered Jan 22 '19 at 11:12

Radoslav Mitev

50
4

thanks! I do need to remember to delete it before I pop it. and I will change it to new. – Its_a_me Jan 22 '19 at 11:16
yea I read that a lot. smart pointers will make it so I dont need to manually delete stuff right? (unless I allocated a 2d array) – Its_a_me Jan 22 '19 at 11:24

Problem with std::vector and/or polymorphism

2 Answers2