0

I am trying to write a code that keeps track of instances of my class. Each instance is uniquely identified by a type (int). I would like to have some kind a map which links a type to a instantiation of my class.

My idea was to use a static map for this, and every instantiation registers itself when the constructor is called:

#include <unordered_map>
#include <iostream>

class Dummy {
    public:
    Dummy(double a, double b, double c, int type);
   
    void GetContents() {std::cout << type_ <<": a: " << a_ << " b: " << b_ << ", c: " << c_ << std::endl;}
    private:
        const double a_;
        const double b_;
        const double c_;
        const int type_;
};

static std::unordered_map<int, Dummy> Type_Map{{12, Dummy(1, 2, 3, 12)}}; // pre-defined instantiation

Dummy::Dummy(double a, double b, double c, int type) : a_(a), b_(b), c_(c), type_(type) {
        Type_Map.insert({type, *this});
    };

In this case, everything seems to work well:

int main()
{
    Dummy(12, 6, 23, 3);
    Dummy(8, 22, -4, 7);
    for (auto a : Type_Map) {
        std::cout << a.first << ", ";
        a.second.GetContents();
    }

}

which gives a correct output:

3, 3: a: 12 b: 6, c: 23
7, 7: a: 8 b: 22, c: -4
12, 12: a: 1 b: 2, c: 3

However, outside of this minimal example and in my real-life project, this seems to be unstable. While the instantiation is still registered in the map inside of the constructor, the map entry is lost as soon as I leave the constructor.

I believe this may have to do with the *this call inside the constructor where the object is not yet correctly initialized? Is this ill-defined behavior?

Is there a better concept that achieves what I want to do?

PS: What I actually need

The code is written this way because I have some instantiations of my StaticData class and many instantiations of my DynamicData class. The content of StaticData instantiations do not change (all members are const), and there are only a few instantiations (this corresponds to the Dummy class in my minimal example).

However, there are a lot of DynamicData instantiations. For every DynamicData instantiation, there is a specific StaticData object which belongs to it. I want to avoid that every DynamicData object has to carry a StaticData object. Therefore, it only carries an id that links it uniquely to a StaticData object. If the user wants to find out which StaticData belongs to my specific DynamicData object, we can just look in the map (this will not happen regularly during runtime, but we still need to keep the information).

The obvious solution would be to just use pointers. However, I want the user to be able to just initialize a DynamicData object with the id instead of having to pass a pointer to a StaticData object.

Fubini
  • 59
  • 1
  • 7
  • 2
    Your map keeps not track of your original instances, instead each of the objects create a new one inside the map. That is not what you want I believe! – Klaus Nov 11 '21 at 16:44
  • @Klaus All members of my class are const, I forgot to mention this (and adapted my code example). So the instances inside the map and the one I construct are not the same, but they are basically identical. – Fubini Nov 11 '21 at 16:49
  • All members are static? Sorry but that makes even less sense. Please provide a [mcve] of the case that fails – 463035818_is_not_an_ai Nov 11 '21 at 16:51
  • @463035818_is_not_a_number Sorry, that was a typo, I meant const. – Fubini Nov 11 '21 at 16:51
  • if that is your code, one problem is that `Dummy` can neither be moved nor copied – 463035818_is_not_an_ai Nov 11 '21 at 16:57
  • @Fubini: "*However, I want the user to be able to just initialize a DynamicData object with the id instead of having to pass a pointer to a StaticData object.*" A pointer *is an ID*; it's just not a number. At some point, someone created a `StaticData` object. So that person had to store an ID of it. Which means that that person could have stored a *pointer* instead of an ID. The only reason I can see to use a number is if a user gets to define what that number is. But that would allow the user to create two such objects with the same value, which is antithetical to what you're trying to do. – Nicol Bolas Nov 11 '21 at 16:57
  • You did not catch the problem! Your map create new instances by every call to `insert`. Simply because the data type is `Dummy` and not pointer or reference type or any proxy. – Klaus Nov 11 '21 at 16:57
  • @NicolBolas Yes, the idea is that the user can decide what ID he wants to use. In my specific case, the ID corresponds to a elementary particle (and they have specific particle codes). So the user basically knows, for example, that the ID 11 corresponds to an electron. If we wants to initialize a `DynamicData` object which describes an electron, he just needs to remember `11`. Constructing two objects with the same ID (that are otherwise not identical) is disallowed and would throw an exception. – Fubini Nov 11 '21 at 17:03
  • @Klaus Yes, it will create a new instance, so far I understand (I believe). But the content of the instantiation would be identical, right? – Fubini Nov 11 '21 at 17:05
  • As long as your copy constructor is valid and all members are initialized before calling the insert method from the map. – Klaus Nov 11 '21 at 17:27
  • If this may help check my answer to [c++ storing an object into an array of objects within the constructor of that object](https://stackoverflow.com/a/25880720/3972710), you get the full MVCE from the coliru link at the end of the answer. – NGI Nov 11 '21 at 22:11

2 Answers2

1

What you want is a registry, a specific object where you create all of the StaticData objects you will ever intend to use (along with their associated IDs). If someone needs a particular StaticData configuration, they add it to the registry. Other code can reference objects in the registry by ID.

The important point is that they do not reference them by constructing an object. They never make a StaticData; they ask the registry for a pointer/reference to one which already exists. The lifetime of all such objects is controlled and managed in the registry, and nobody else gets to manage such things.

You're trying to make the registry implicit by hiding it behind StaticData's constructor. That is fertile ground for creating lifetime issues, as the ownership relationship between the StaticData you create and the one stored in the registry is... murky. It's best to make it explicit by directly asking the registry for a reference to the object. It makes it clear that you get to access the object, but you don't own it.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
1

Your problem is that the static map is filled inside the constructor, but outside of the constructor, the entry is gone. It seems like there are two different instances of the static map. You see one instance inside the constructor, and another instance outside of the constructor.

It looks like, you define your static map in a header file. Each .cpp file that includes the header file will "see" its own instance of the static map. Solution: Make a class which keeps the map as a static member. If this does not solve your problem, make also sure that you define the static member inside a .cpp file, like:

map<int, Dummy> TypeRegistry::Type_Map;

eberhard
  • 86
  • 4
  • Technically, this has been the solution of my problem! In my project, the map was not a member of a class. – Fubini Nov 16 '21 at 17:11