Home > other >  Issue when calling template function that returns a pointer to base class
Issue when calling template function that returns a pointer to base class

Time:05-26

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?

CodePudding user response:

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

CodePudding user response:

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.

  • Related