Accessing a static std::set with accessors, is this wise or should I just access it directly?

Question

This question follows on from the outcome of this one: How to automatically maintain a list of class instances?

With reference to the previous question, I created my static list of objects static std::set< Object* > objects;

However, to avoid circular referencing between Engine and Object, I moved it out of Engine and into a separate header file. I then realised that rather than directly interacting with the list, I could use a bunch of static accessors. This way if anything is making changes to the list, I can always break on these functions.

Are there any other benefits for doing it this way? Or is this a bad way to go? I never intend to instantiate ObjectManager ever - should I be using what I believe are called 'free functions' instead to manage this std::set, with no class?

I have created a test project to keep things simple in testing this out. The Inheritor class inherits from the Object class. The code for the ObjectManager class (referenced by Main.cpp, Object and Inheritor in this case, but a very similar one would be used in my main project) is below:

ObjectManager.h:

#include <set>

class ObjectManager
{
    friend class Object;
    friend class Inheritor;

public:
    static int ObjectCount();
    static void AddObject(Object *);
    static void RemoveObject(Object *);

    static int InheritorCount();
    static void AddInheritor(Inheritor *);
    static void RemoveInheritor(Inheritor *);

    static std::set<Object *>::iterator GetObjectListBegin();
    static std::set<Object *>::iterator GetObjectListEnd();

    static std::set<Inheritor *>::iterator GetInheritorListBegin();
    static std::set<Inheritor *>::iterator GetInheritorListEnd();
private:
    ObjectManager();
    ~ObjectManager();
};

ObjectManager.cpp:

#include "ObjectManager.h"

static std::set<Object *> objectList;
static std::set<Inheritor *> inheritorList;

ObjectManager::ObjectManager()
{

}
ObjectManager::~ObjectManager()
{
}

int ObjectManager::ObjectCount()
{
    return objectList.size();
}

void ObjectManager::AddObject(Object *input)
{
    objectList.insert(input);
}

void ObjectManager::RemoveObject(Object *input)
{
    objectList.erase(input);
}


int ObjectManager::InheritorCount()
{
    return inheritorList.size();
}

void ObjectManager::AddInheritor(Inheritor *input)
{
    inheritorList.insert(input);
}

void ObjectManager::RemoveInheritor(Inheritor *input)
{
    inheritorList.erase(input);
}

std::set<Object *>::iterator ObjectManager::GetObjectListBegin()
{
    return objectList.begin();
}

std::set<Object *>::iterator ObjectManager::GetObjectListEnd()
{
    return objectList.end();
}

std::set<Inheritor *>::iterator ObjectManager::GetInheritorListBegin()
{
    return inheritorList.begin();
}

std::set<Inheritor *>::iterator ObjectManager::GetInheritorListEnd()
{
    return inheritorList.end();
}

**

EDIT:

** I have rewritten my code to remove the need for an ObjectManager.

Instead of using ObjectManager, each class I want a list of contains the static list in its source file, so Object.cpp contains static std::set<Object *> objectList; and Inheritor.cpp contains static std::set<Inheritor *> inheritorList;.

Then, each of those two classes contains a static Count() function, for getting the number of items in its respective list, GetListBegin() and GetListEnd() for getting the beginning and end of the set.

As the functions share the same name in both the base class and the derived one, Object::Count() gets the number of Object instances in its respective list, and Inheritor::Count() gets the number of Inheritor instances in its respective list.

I don't know if this is a bad way of doing this or not. Nothing can add or remove from the list outside of each respective class. My only issue is I'm not sure how to stop the static functions from being available in anything that say, inherits from Inheritor for example.

Answer 1

If std::set provides exactly the right interface then you can use it directly. If it doesn't (and that's usually the case: it has a very broad interface) then you should write a class or a set of functions that provide the interface you need, and implement appropriately, perhaps with std::set, perhaps with something else.

Answer 2

I would put this set as a private static member of Object class, which adds itself to the set in the constructor (don't forget copy constructor, if permissible). Then make a public read-only accessor to the set (return const reference).

I don't understand the meaning of your Inheritor set.

If this is a list of live objects in a game engine, I would make it a non-static member of engine class which keeps track of all its objects.