0

could somebody please explain why this happens? I have written the following:

1) a WindowManager class which is implemented as a singleton and has an Instance() method, defined like so:

static WindowManager * instance_;
//...
WindowManager * WindowManager::instance_ = 0;
WindowManager & WindowManager::Instance()
{
    if (!instance_)
        instance_ = new WindowManager();
    return *instance_;
}

2) a WindowManager::createWindow method that returns a reference to a newly created window, defined like so:

Window & WindowManager::createWindow()
{
    windows_.push_back(Window());
    return windows_.at(windows_.size() - 1);
}

3) a Window::print method that prints a message inside the window

In my main program, I have written the following:

ui::Window & win1 = ui::WindowManager::Instance().createWindow();
ui::Window & win2 = ui::WindowManager::Instance().createWindow();
win1.print("First window");
win2.print("Second window");

This does not work! Only the second call to print is executed (for win2). However, if I change the order, like so:

ui::Window & win1 = ui::WindowManager::Instance().createWindow();
win1.print("First window");
ui::Window & win2 = ui::WindowManager::Instance().createWindow();
win2.print("Second window");

then everything works as expected. If anybody could shed some light on this situation, any help would be greatly appreciated.

Dan Nestor
  • 2,441
  • 1
  • 24
  • 45
  • The problem was that in `createWindow` I return a reference to an object created on the stack, which is destroyed when the method ends. Changing `windows_` from `std::vector` to `std::vector` and modifying everything else accordingly solved my problem. – Dan Nestor Oct 28 '11 at 00:15
  • Thats not your cause, but that would solve the real problem – Mooing Duck Oct 28 '11 at 01:15
  • Please read this: http://stackoverflow.com/questions/1008019/c-singleton-design-pattern/1008289#1008289 – Martin York Oct 28 '11 at 03:06

1 Answers1

2

Here you have quick example which illustrates the problem:

#include <iostream>
#include <vector>
using namespace std;

struct T
{
    int id;
    T(int id) : id(id)
       { cout << "created " << id << endl; }
    T(T const& t) : id(t.id)
       { cout << "copy: " << t.id << endl; }
    void print(char const* m)
       { cout << id << ": " << m << endl; }
};

vector<T> ts;

T& create(int id)
{
    ts.push_back(T(id));
    return ts.at(ts.size() - 1);
}

int main()
{
    // Uncomment these lines and compare results
    //int const max_windows = 10;
    //ts.reserve(max_windows);

    T& t1 = create(1);
    T& t2 = create(2);
    t1.print("t1");
    t2.print("t2");
}

Compile and run as it is and see what is printed to stdout. Then uncomment reserve() call, compile and run once again and compare with previous output.

The problem is that std::vector::push_back causes reallocation of the data internally. This invalidates all references, pointers or iterators to vector elements you've obtained before the reallocation.

mloskot
  • 37,086
  • 11
  • 109
  • 136
  • 2
    Std::deque does not suffer from this problem, and is nearly as fast as a vector for most purposes. – Mooing Duck Oct 28 '11 at 01:16
  • Thanks! In the meantime, I managed to work around the problem by changing my vector to hold pointers to objects. What is the preferred c++ way to hold a series of objects inside a manager class? A container of objects, or a container of pointers to objects? – Dan Nestor Oct 28 '11 at 01:19
  • See @MooingDuck comment. Also, you can have a hybrid: collection of shared_ptr. – mloskot Oct 28 '11 at 12:33
  • Note that std::deque _will_ still have this problem if you erase any elements from the middle. Std::list/set/map ensure that references are not invalidated ever. – Mooing Duck Oct 28 '11 at 15:00