A lot of times I'm using this kind of pattern in my code:


class Object
{
public:
Object() : m_pool(NULL) {}
~Object()
{
if(m_pool) m_pool->unregister(*this);
}
void create(ObjectPool& pool, ...)
{
m_pool = &pool;
m_pool->register(*this);
....
}
private:
ObjectPool* m_pool;
};
class ObjectPool
{
public:
void unregister(Object& object)
{
objects.erase(std::remove(objects.begin(), objects.end(), &object), objects.end());
}
void register(Object& object)
{
objects.push_back(&object);
}
private:
std::vector<Object*> objects;
};


Where ObjectPool is some class that does something with all the registered objects (I would call it a manager but I won't ;-)).

I like this pattern because you can be sure the object removes itself from the pool on destruction.
But I also dislike the pattern because you can never be sure if m_pool is still valid when the object is destroyed.

1. Is this a bad or even anti pattern? I've seen variations of this in popular engines but those were also using singletons and other "no-nos".
2. If not, is there a better way to implement this so that one doesn't have to worry about the destruction order?

If you're worried about the lifetime of the pool, you can store a smart pointer of some type rather than a raw pointer. Depending on your goals a boost::shared_ptr or weak_ptr may work.

Oh yeah, that works like a charm. So the pattern itself is not bad practice?

Part of writing correctly working software is having a good design, enforcing that this design is followed. If your design requires every instance of A to be registered in B, then it makes sense to register A within the constructor and unregister in the destructor. This way your class enforces the correctness of it's state, and you don't rely on other classes to enforce your correct state. It will become a lot easier to debug problems if every class it responsible for being valid, I know from experience with working with systems that followed the principle to initialize nothing in the constructor and relying on other pieces of code to make the object valid that it is extremely difficult to debug bugs as you have no idea where to look. Imagine this: to create a valid object of type 'A' you need to call some init function on it for it to become valid, but somebody forgot to call this function. Where do you look? You can't put a breakpoint in a function that you forgot to call, so it actually means going through many lines of code in many different classes to find the error.

About your concern of lifetime: if your design requires that every object of type A is registered with an object of type B, then it should not be allowed for object B to be destroyed while it still has references to object A. If it does, then you have a serious bug. I personally recommand against fancy stuff as smart pointers in most of these situations, all they do is take away the symptom of a serious flaw in your program, they do not fix it. And careless use of smart pointers can lead to bugs as well which are very painfull to debug. Assertions might seem deadly as they stop your application immediately when they fail, but it will safe a lot more debugging time in the end.

So in this particular example, it really depends on if your Object can exist without an ObjectPool or not. If it can exist without it, then the pool could set the pool pointer to null in every Object it knows about when it is destroyed. In case an Object cannot exist without an ObjectPool you should move the registering part to the constructor instead, and ObjectPool could have an assertion that it has no objects left. To be totaly safe your design is flawless (as your register function are public and should be public, as you should avoid friend classes), register should do a check if the given object does indeed belong to this pool and if not already in the list. Only a couple of weeks ago I had to debug a very nasty bug involving a pool where somebody was inserting an object into twice and it totally started to crash the whole application a few frames later with no way of telling where it went wrong. After I added an assertion for inserting an object into the pool twice I was able to fix this bug within a minute.

Thanks for the detailed reply. You have covered many of my concerns and I'm glad to hear that this is not such an unusual thing to do. I've settled for the solution to pass the pool to the constructor and keep a reference.