0

My goal is to create many objects of derived classes and store them in a std::map with a std::string as a key and the pointer to that object as the value. Later in the flow, I access all the keys and values and call some virtual functions re-implemented in the derived classes.

I ended up in a situation where I had to call a template within a template.

Model.h

#include<Base.h>
class myClass {

  template<typename T>
  Base* createT() { new T; }

  typedef std::map<std::string, Base*(*)()> map_type;
  static map_type* getMap() {
    if (!map) {
      map = new map_type;
    }
    return map;
  }

  template<typename T>
  void registerT(std::string& s) {
     getMap()->insert(std::make_pair(s, createT<T>())); // problem is here 1 of 2
  }
};

Model.cc

#include <Model.h>
#include <DerivedA.h>

registerT<DerivedA>("DerivedA"); // problem is here 2 of 2
registerT<DerivedB>("DerivedB");
  
// To be implemented getValue(). Eventual goal is this.
auto objA = getValue("DerivedA"); 
objA->init(); // virtual
objA->run();  // virtual

The createT is supposed to create an object and return me the pointer. But it is not working and the compiler is throwing this error:

error: cannot call member function ‘HB::Base* myClass::createT() [with T = HB::DerivedA]’ without object
      getMap()->insert(std::make_pair(s, createT<R>()));
                           ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~

What am I doing wrong?

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
vijaychsk
  • 39
  • 5
  • 1
    Is `createT` supposed to be static? Also why are you calling the function? Don't you want take its address instead? (Make it static unless you want to deal with pointers to members) Anyway this whole design is a huge red flag for me. If you start writing types as strings, you most likely made a wrong turn somewhere. – Quimby May 25 '22 at 18:39
  • `std::make_pair())` `s` is a reference to `std::string`; it cannot be used as a template argument where a type is expected... – fabian May 25 '22 at 18:44
  • It seems you misunderstand something basic about templates. Please invest in [some good C++ books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list/388282#388282) and read more about them. – Some programmer dude May 25 '22 at 18:45
  • Also, the error doesn't match your shown code. Please make sure to show us a proper [mre], and the full build-log from that example. – Some programmer dude May 25 '22 at 18:47
  • @Quimby I'm calling `createT` to create an object for any type `T`, usually my derived classes. Can you recommend a better design? I'm new and it is possible my design can be wrong. My goal is maintain map of objects which I can access through string or some int or enum value). – vijaychsk May 25 '22 at 18:47
  • @fabian typo corrected. I'm not passing string as type but argument to `make_pair` – vijaychsk May 25 '22 at 18:54

2 Answers2

0

Your map expects pointers to free-standing functions, but your createT() is a non-static class method, so it is not compatible (also, your createT() is not actually return'ing anything).

When insert()'ing into your map, you are calling createT() and then trying to insert its returned object pointer, when you should instead be inserting the address of createT() itself, eg:

Model.h

#include <Base.h>
#include <memory>

class myClass {

  template<typename T>
  static std::unique_ptr<Base> createT() { return std::make_unique<T>(); }

  typedef std::map<std::string, std::unique_ptr<Base>(*)()> map_type;
  static map_type& getMap() {
    static map_type instance;
    return instance;
  }

  template<typename T>
  static void registerT(const std::string& s) {
     getMap().emplace(s, &myClass::createT<T>);
  }

  static auto getValue(const std::string& s) {
     return getMap()[s];
  }
};

Model.cc

#include <Model.h>
#include <DerivedA.h>

myClass::registerT<DerivedA>("DerivedA");
myClass::registerT<DerivedB>("DerivedB");
  
auto createA = myClass::getValue("DerivedA"); 
auto objA = createA();
objA->init(); // virtual
objA->run();  // virtual

However, you said in your description that you want to store object pointers in the map, not function pointers. Your code does not match your description. In which case, you would need something more like this instead:

Model.h

#include <Base.h>
#include <memory>

class myClass {

  template<typename T>
  static std::unique_ptr<Base> createT() { return std::make_unique<T>(); }

  typedef std::map<std::string, std::unique_ptr<Base>> map_type;
  static map_type& getMap() {
    static map_type instance;
    return instance;
  }

  template<typename T>
  static void registerT(const std::string& s) {
     getMap().emplace(s, createT<T>());
  }

  static auto& getValue(const std::string& s) {
     return getMap()[s];
  }
};

Model.cc

#include <Model.h>
#include <DerivedA.h>

myClass::registerT<DerivedA>("DerivedA");
myClass::registerT<DerivedB>("DerivedB");
  
auto &objA = myClass::getValue("DerivedA"); 
objA->init(); // virtual
objA->run();  // virtual
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Thank you for the comment. What is advisable to store in map `object pointer` or `function pointer`? I may be wrong when I asked that question because I'm still in the learning phase. However my goal is clear, I need to run member functions from each of the derived classes after they are registered. – vijaychsk May 25 '22 at 21:28
  • "*my goal is clear*" - actually, it is not. Do you want a `map` of functions that create new objects on-demand, or do you want a `map` of objects that have already been created? Pretty different requirements. – Remy Lebeau May 25 '22 at 21:48
  • The latter -- map of object that have already been created. So that I can simply call various methods on demand by simply taking the object that's in the map. – vijaychsk May 25 '22 at 22:13
  • OK, so then use the 2nd set of code I showed you – Remy Lebeau May 25 '22 at 22:30
0

createT<T>() is a invokation of the function; you don't take a function pointer here.

There are some other things in the code that are suboptimal:

  • References should be preferred to pointers. You could easily replace the creation of the map via new operator with a magic static. (You're missing the declaration of a variable named map anyways.)
  • Returning a pointer leaves the ownership unclear. Return a std::unique_ptr<Base> instead of Base* and you'll have a much easier time avoiding resource leaks.

Here's code using free functions allowing you to use the desired code.

struct Base
{
    virtual ~Base() = default;

    virtual void init() = 0;
    virtual void run() = 0;
};

struct DerivedA : Base
{

    void init() override
    {
        std::cout << "DerivedA::init()\n";
    }
    void run() override
    {
        std::cout << "DerivedA::run()\n";
    }
};

struct DerivedB : Base
{

    void init() override
    {
        std::cout << "DerivedB::init()\n";
    }
    void run() override
    {
        std::cout << "DerivedB::run()\n";
    }
};


template<class T>
std::unique_ptr<Base> Create()
{
    return std::make_unique<T>();
}


template<class T>
void registerT(std::string&& key);

std::unique_ptr<Base> getValue(std::string const&);

/**
 * Type used for restricting access to the Registrar data
 */
class RegistrarHolder
{

    static std::map<std::string, std::unique_ptr<Base>(*)()>& Registrar()
    {
        static std::map<std::string, std::unique_ptr<Base>(*)()> registrar;
        return registrar;
    }

    template<class T>
    friend void registerT(std::string&& key);

    friend std::unique_ptr<Base> getValue(std::string const&);
};

template<class T>
void registerT(std::string&& key)
{
    RegistrarHolder::Registrar().emplace(std::move(key), &Create<T>);
}

std::unique_ptr<Base> getValue(std::string const& key)
{
    return (RegistrarHolder::Registrar().at(key))();
}

int main() {

    registerT<DerivedA>("DerivedA");
    registerT<DerivedB>("DerivedB");

    auto objA = getValue("DerivedA");
    objA->init();
    objA->run();

    auto objB = getValue("DerivedB");
    objB->init();
    objB->run();

    return 0;
}

Note that this implementation assumes you want to create a new object with every call to getValue, even if you've passed the same parameter to the function before.

fabian
  • 80,457
  • 12
  • 86
  • 114