C++ : Is it bad practice to use a static container in a class to contain pointers to all its objects for ease of access?

Question

I would like to know if it's bad practice to have a static container in a class to store all the pointers to the class' objects so they can all be easily accessed by the base classes of the program. This is for a game and I saw it on sdltutorials dot com, and I find it very useful. It would allow me to have a very neat structure for my game and I don't really see a downside doing this, but I know I have to be careful with "global" access and maybe there's a negative effect I'm not seeing right now.

Here is the context/example. The game has a base class with basic methods such as Loop(), Render(), PlayAudio(), CleanMemory(). The idea is to have individual objects to have the same methods being executed inside the base method. Example in pseudocode:

Game::Render() {

  for (iterate all enemies in static container) {

     current_enemy::Render();
  }
}

To be sure, the static member inside the class would look like this:

static std::vector<Enemy*>    EnemyList;

So this way, when your game is executing the base Render() method, for example, you can iterate all the enemies in the enemies' class static container and execute all their individual Render() methods, then do the same for environment objects, then for the player, etc.

I would just like to make sure I'm aware of any downside/complication/limitation I might encounter if I choose this method to build my game, because I don't see any right now but I know a have to be careful with static and global stuff.

Thanks very much for your time.

Answer 1

It is certainly convenient, however a static variable or a Singleton are nothing more than global variables; and having global variables comes with drawbacks:

• the dependencies of a function become unclear: which global does it rely upon ?
• the re-entrancy of a function is compromised: what if current_enemy.render() accidentally calls Game::Render() ? it's an infinite recursion!
• the thread-safety of a function is compromised, unless proper synchronization takes place, in which case the serialized access to the global variable bog down the performance of your concurrent code (because of Amdahl's Law)

It might seem painful and pointless to explicitly pass a reference to an instance of Game wherever you need to, however it leaves a clear path of dependencies that can be followed and as the software grows you will appreciate explicitness.

And there is, of course, much to be said about transforming the program to have two instances of Game. While it might seem incongruous in this precise situation, in general it is wise not to assume that it will never be necessary in the future, for we are no oracles.

Answer 2

Different people may have different opinions about this. I can give you some advice on how to store your static objects in a better way.

Use the singleton pattern for a class which stores your objects:

class ObjectManager
{
private:
   std::vector<Enemy*> enemies_;
   std::vector<Friend*> friends_;
   ...
public:
   void add(Enemy* e) { enemies_.push_back(e); }
   ...
   const std::vector<Enemy*> enemies() const { return enmies_; }
   ...
private:
   static ObjectManager* instance_;
public:
   static ObjectManager* Get() { return instance_; }
   static void Initialize() { instance_ = new ObjectManager(); }
}

You can access it like that (example with C++11 ranged-based for):

void Game::Render() {
    for(auto e : ObjectManager::Get()->enemies()) {
        e->Render();
    }
}

This is especially convenient for subclasses which want to access information about the world. Normally you would have to give a pointer to ObjectManager to everyone. But if you have only one ObjectManager anyway the singleton pattern may remove clutter from your code.

Don't forget to create the singleton at the beginning of your program by calling ObjectManager::Initialize();.

Answer 3

I would not suggest doing this the way you are. At this point you may as well have a bare global variable in a namespace, it is the same thing you are doing right now.

I also do not suggest using singletons.

When should the Singleton pattern NOT be used? (Besides the obvious)

The best way to approach things is to do good old parameter passing (dependency injection) wherever possible. With careful design this is feasible system wide, and it avoids all the problems you have with globally accessible resources.

When you don't have the luxury of designing your system in such a way, and you are working within existing code that already has quite a bit of trouble with singleton dependence, or loss of locality between resources several levels removed from where they are needed (and you cannot afford to modify the interfaces to cascade dependencies downward) this may not be useful advice.

A middle-ground between bare global and singleton is the service-locator. Many people still consider service-locator an anti-pattern, but most people also agree that it is less bad than the singleton since it offers a certain level of abstraction and decouples creation from supplying the object which means you can offer up a derived class easily if your design or environment changes.

Here is a description of the pattern:

http://gameprogrammingpatterns.com/service-locator.html

And here is a discussion about the singleton vs service-locator.

If Singletons are bad then why is a Service Container good?.

I like the highest voted (but not accepted) answer best.