3

I'm currently working on a 2D game engine and I've read about auto_ptr's and how you should never put them in standard containers.

My engine has this structure:

StateManager -- has many --> State's.

States are created and allocated in main, outside of the engine. I want the engine to store a list/vector of all the states so I can change between them on command.

For example: 

SomeState *test = new SomeState();
StateManager->registerState(test);

Since states die when and only when the application dies, can I use this approach?

std::auto_ptr<SomeState> test(new SomeState());
StateManager->registerState(test.get());

// Inside StateManager State *activeState; // State manager then maintains a vector std::vector<State*> stateList; // and upon registerState it adds the pointer to the vector void registerState(State *state) { stateList.push_back(test); }

The StateManager should only maintain a vector of pointers to the states, it shouldn't need to take ownership. When I want to change state in the engine, I just change the activeState pointer to point to the desired state which is found in the stateList vector.

Is this a bad approach?

John
  • 2,571
  • 6
  • 32
  • 40

5 Answers5

9

Why not use std::vector<State> instead and not worry about storing pointers? It sounds like the list of states is fixed so any iterators will not be invalidated during the lifetime of the program.

Alternatively, I've found the Boost Pointer Container library to be useful for this sort of thing.

Ferruccio
  • 98,941
  • 38
  • 226
  • 299
  • When using boost containers, would I just pass the auto_ptr as is to registerState() and change the parameter type to std::auto_ptr, or do I still pass it by using the .get() function? – John Jul 13 '10 at 12:03
  • 3
    You don't use auto_ptrs with boost::ptr_vector. You just hand it a naked pointer and the ptr_vector takes ownership of it. – Ferruccio Jul 13 '10 at 12:07
  • Thank you for clearing that up. I'll take a look at Boost Pointer Containers as you suggested. – John Jul 13 '10 at 12:13
4

Since StateManager doesn't own contained objects, its contained type should be State*, and not smart pointer. It's OK to use auto_ptr in the main function, this is not related to the StateManager container.

In the case you want to have container which is responsible for contained objects lifetime, you can use smart pointer as contained type. But using auto_ptr is dangerous for this, since it has primitive ownership algorithm. It is better to use shared_ptr in such containers.

Alex F
  • 42,307
  • 41
  • 144
  • 212
  • +1 Because there is no need to pay for either managing smart pointers or copying objects in this situation. Normal pointers are OK. – Tomek Szpakowicz Jul 13 '10 at 12:11
  • Maybe you could/would like to elaborate on why using `auto_ptr` could be dangerous? (When is the primitive ownership a problem?) – Kissaki Oct 06 '11 at 07:38
3

If there is nothing that precludes it (polymorphism etc.), then I'd just use the simplest approach, namely std::vector<State>. activeState can still be a pointer (or an iterator).

Philipp
  • 48,066
  • 12
  • 84
  • 109
1

The way you use auto_ptr in the code you posted is not going to work.

std::auto_ptr<SomeState> test(new SomeState());
StateManager->registerState(test.get());

When the "test" variable goes out of scope, its' destructor will be called which will delete the SomeState object you just registered with StateManager. So now your StateManager is keeping a pointer to a deleted object, which will probably lead to a crash down the way.

Instead, you could look into using a reference counter smart pointer type, e.g. boost::shared_ptr.

Haakon
  • 295
  • 1
  • 12
  • 1
    Right now, the StateManager is deleted before the states since the states and the statemanager should be destructed at the same time even if this means invoking release() on the state auto_ptr's in main() -- right? – John Jul 13 '10 at 11:55
  • I'm not entirely sure what you mean, but if you're saying that the test variable in the code you posted will not go out of scope before the StateManager is deleted, then there is no problem and your approach should work just fine. – Haakon Jul 13 '10 at 12:06
  • So, “is not going to work” is incorrect. Rather, it’s *dangerous* because of the potential issue you mentioned. Another place/var/destruction order you’ll have to keep in mind instead of the code managing for you (which is the idea of the auto_ptr). – Kissaki Oct 06 '11 at 07:42
0

auto_ptr s are supposed to manage the object's life time within a specified context. e.g.

void foo() { MyObject* pNewObject = new MyObejct;

// do something delete pNewObject; }

In the above function, if some exception occurred in //Do Something code, the object will not be free d and you will face memory leak in the code. We use Auto_ptrs in this kind of situations.

But it's not suitable for the following context (releasing an array). In the destructor of auto_ptr, it calls delete ptr; but with arrays we have to call delete[] ptr; If you use auto_ptr with array, memory leak will be occurred and leads to unexpected behavior void foo() {

unsigned char* pStream = new unsigned char[size]; ....

delete[] pStream; }

If you don't have much state and it's not so greedy for memory I recommend to use Singleton objects for your state classes. So you don't have to worry about memory management. State manager can handle the active state and the state objects but I don't think it's a good idea to Initialize the state object outside state manager and do the rest in statemanager and somewhere else. please ensure that the creation and destruction are unified

sarat
  • 10,512
  • 7
  • 43
  • 74
  • Initializing objects outside of the class that depends on them actually is (or at least can be) a good idea. For loose coupling and/by dependency injection. – Kissaki Oct 06 '11 at 07:46
  • Also, I don’t quite get what your point with the text prior to last paragraph is. Neither does it lead or elaborate on your point (last paragraph), nor does it answer the question. – Kissaki Oct 06 '11 at 07:47