Use a map across multiple objects

Question

I use global maps to register one or several objects of the same type. I started with using a global namespace for that purpose.

Take this for an example (code untested, just an example):

//frame.h
class frame
{
public:
  frame(int id);
  ~frame();
};

namespace frame_globals
{
  extern std::map<int, frame *> global_frameMap;
};

//frame.cpp
#include "frame.h"
namespace frame_globals
{
  std::map<int, frame *> global_frameMap;
}

frame::frame(int id)
{ 
  //[...]
  //assuming this class is exclusively used with "new"!
  frame_globals::global_frameMap[id] = this;
}

frame::~frame()
{
  frame_globals::global_frameMap.erase(id);
}

This was a rather quick solution I used, but now I stumble again on the use case, where I need my objects registered and I asked myself if there was no better way, than using a global variable (I would like to get rid of that).

[EDIT: Seems to be incorrect] A static member variable is (to my best knowledge) no option, because a static is inline in every object and I need it across all objects.

What is the best way to do this? Using a common parent comes to mind, but I want easy access to my frame class. How would you solve it?

Or do you know of a better way to register an object, so that I can get a pointer to it from somewhere else? So I might not exlusively need "new" and save "this"?

Answer 1

I would consider reversing the design. eg. what is wrong with using language built functionality ?

std::map<int, frame> frames_;

frames_.emplace(id);//adding, ie. creating frames

frames_.erase(id)//removing, ie. deleting frames

Now creating/deleting is as easy as before. Coincidently, if someone needs a frame for a different purpose there is no issue here. If frame is supposed to be polymorphic you could store std::unique_ptr instead.

Also I would consider making id a member of frame and then store in a std::set instead of std::map.

class frame
{
public:
  int id;
  frame(int id);

  friend bool operator <(const frame& lhs, const frame& rhs) {return lhs.id < rhs.id;}
};

std::set<frame> frames_;

Deleting is just:

frames_.erase(frame);

However in order to lookup based on id is now somewhat more complicated. Fortunately theres a solution at hand. That requires implementing a compare that is transparent, which among other things involves defining is_transparent.

Overall you need to think about ownership. Ask yourself: where or who would own/store the map/frame pointers/etc. Store it there. In case of shared ownership, use a shared_ptr (but use only this pattern when actually needed - which is more rare than what people use it for).

---

Some relevant core-guidelines

I.3 Avoid singletons

R.5 and R.6 Avoid non-const global variables

R.11 Avoid calling new and delete explicitly

R.20 and R.21 Prefer unique_ptr over shared_ptr unless you need to share ownership

Answer 2

You should ideally have a factory pattern to create a delete frame objects. The constructor of the frame must not be managing the map. It's like citizens are manging their passports (ideally govt does that for citizens). Have a manager class to keep frame in map (std::map<int,frame>). A method provided by the manager class would create the object:

class FrameManager
{
    std::map<int,frame> Frames;
public:
    frame CreateFrame(int id) 
    {
           frame new_frame(id);
           Frames[id] =  new_frame;
    }      
};

What about removals? Well, depending on your design and requirement, you can have either/both of:

• Removal of frames by the destructor of the frame. It may call FrameManager::Remove(id);
• Allow removal also by FrameManager only (FrameManager::Remove(id)). I'd choose this (see more below).

Now, note that with this approach, many objects so frame will get created (local, assignment to map, return etc.). You may use shared_ptr<frame> as the return type from CreateFrame, and keep shared_ptr as the type of map map<int, shared_ptr<frame>>. It may seem complicated to use shared/unique_ptr but they become really helpful. You don't need to manage the lifetime. You may pass same shared_ptr<frame> to multiple functions, without creating frame object multiple times.

Modified version:

class FrameManager
{
    std::map<int, shared_ptr<frame>> Frames;
public:
    shared_ptr<frame> CreateFrame(int id) 
    {
        if(Frames.count(id)>0) return nullptr;  // Caller can check shared_ptr state

        shared_ptr<frame> new_frame = make_shared<frame>(id);
        Frames[id] =  new_frame;

        return new_frame;
    }      
};

Using a frame-manager will allow complete control over frames creation, better error handling, safe/secure code, safe multithreaded code.

Answer 3

I would like to put few points base on code posted by you.

1. How some one will know about frame_globals::global_frameMap and what it does with out looking source code.
2. If it is a global object then frame class logic can be bypassed and global object can be modified from outside.
3. What if someone wants to mange frame objects in different way or if in future some requirement changes for example if you want to allow duplicate frames then you have to change frame object.
4. frame class is not only having frame related data member and member function but also have managing logic. It seems not following single responsibility principle.
5. What about copy/move constructor? std::map/std::set will affect constructors of frame class.

I think logic separation is required between frame and how to manage frames.I have following solution ( I am not aware of all use cases, my solution is based on posted code)

frame class

1. frame class should not have any logic which is not related to frame.
2. If frame object need to know about other frame objects then a reference of ContainerWrapper should be provided (not directly std::map or std::set, so that change will not impact frame)

ContainerWrapper class

1. ContainerWrapper should manage all the frame objects.
2. It can have interface as per the requirement, for example try_emplace which returns iterator of inserted element (similar to std::map::try_emplace), so rather then first creating frame object and then try to insert in std::map, it can check the existence of the id(key) and only create frame object if std::map doesn't have any object with specified id.

About std::map/std::set,
std::set can be used if frame object once created is not required to modify because std::set will not let you modify frame without taking it out and re-inserting it or you have to use smart pointer.

By this way a container object can be shared across multiple frame objects and doesn't have to be global object.