0

I have this code which is an adaptation from How to pass arguments to factory elements constructors? to work with smart pointers.

#include <unordered_map>
#include <string>
#include <iostream>
#include <memory>
 
class Base;

class myFactory
{
public:
    typedef std::unordered_map<std::string, void*> registry_map;
 
    virtual ~myFactory() = default;
 
    static registry_map & registry()
    {
        static registry_map impl;
        return impl;
    }

    template<typename ...T>
    static std::shared_ptr<Base> instantiate(std::string const & name, T&&...args)
    {
        auto it = registry().find(name);
        if ( it == registry().end()) return 0;
        typedef std::shared_ptr<Base> (*create_type)(T...);
        auto create_fun = reinterpret_cast<create_type>(it->second);
        return create_fun(args...);
    }

    template<typename F>
    static bool sign(const std::string& name, F func)
    {
        registry()[name] = reinterpret_cast<void*>(func);
        return true;
    }
};
 
class Base: public myFactory
{
    public:
        virtual void f() = 0;
        virtual ~Base() = default;
};
 
class DerivedExample : public Base
{
private:
    static bool sign;

public:
    DerivedExample(int a, int b){std::cout << a << b << std::endl;}
    DerivedExample() = default;
    
    static std::shared_ptr<Base> create() { return std::make_shared<DerivedExample>();}
    static std::shared_ptr<Base> create(int a, int b) { return std::make_shared<DerivedExample>(a,b);}

    virtual void f() override { std::cout << "DerivedExample" << std::endl; }
};
 
bool DerivedExample::sign = DerivedExample::myFactory::sign("DerivedExample", DerivedExample::create());
bool DerivedExample::sign = DerivedExample::myFactory::sign("DerivedExample", DerivedExample::create(int a, int b)); // redefinition


int main()
{
    std::shared_ptr<Base> p1 = Base::instantiate("DerivedExample");
    std::shared_ptr<Base> p2 = Base::instantiate("DerivedExample", 1, 2);
    p1->f();
    p2->f();

}

This implementation does not let me register 2 constructors for the factory because of a redefinition of signboolean. Is there a way to fix the code in order to register multiple create functions for the the same class?

Best regards

8
  • You don't need to register twice. The idea is to register the possibility to create an object of same class, but you may do that with different creators. Thus, a registration per class will suffice I think. Commented May 27, 2023 at 12:36
  • A way to do that may be to replace variadic arguments for instantiation by a struct inheriting from an abstract parameter struct. Thus you have a single signature and inside the create function you can do different things according to the actual containt of your parameter. May be it's not as smart as using variadics but it's probably simpler, at least for a first try. Commented May 27, 2023 at 12:41
  • The cast is dangerous, especially with forwarding reference. mismatch can happen really easily (mismatching const, reference, promotion/conversion, ...) Commented May 27, 2023 at 13:09
  • @Jarod42 is there a more robust way of doing this? Commented May 27, 2023 at 16:04
  • With std::any, you might catch that, even if... Commented May 28, 2023 at 18:27

2 Answers 2

0

The bool sign is used for "auto" registration at global scope. You might just add extra variable:

class DerivedExample : public Base
{
private:
    static bool sign1;
    static bool sign2;
    // ...
};

bool DerivedExample::sign1 = DerivedExample::myFactory::sign("DerivedExample1", static_cast<std::shared_ptr<Base> (*)()>(&DerivedExample::create));
bool DerivedExample::sign2 = DerivedExample::myFactory::sign("DerivedExample2", static_cast<std::shared_ptr<Base> (*)(int, int)>(&DerivedExample::create));

The way to register was wrong BTW, especially when overloads are involved.

Demo

Sign up to request clarification or add additional context in comments.

1 Comment

Thanks for the suggestion. However, if possible I would like to avoid having to create a new boolean per constructor (It is not very elegant). At the end of the day, I would like to call Base::instantiate(arg1, arg2, argN) and build the correct object. If you do not know of a better/more robust approach I will go with one
0

A way to fix you code:

#include <iostream>
#include <memory>
#include <string>
#include <unordered_map>

class Base;

class myFactory {
   public:
    typedef std::unordered_map<std::string, void*> registry_map;

    virtual ~myFactory() = default;

    static registry_map& registry() {
        static registry_map impl;
        return impl;
    }

    // here needing to pass by value in order to match your create function
    // signature
    template <typename... T>
    static std::shared_ptr<Base> instantiate(std::string const& name,
                                             T... args) {
        auto it = registry().find(name);
        if (it == registry().end()) return 0;
        typedef std::shared_ptr<Base> (*create_type)(T...);
        auto create_fun = reinterpret_cast<create_type>(it->second);
        return create_fun(args...);
    }

    template <typename F>
    static bool sign(const std::string& name, F func) {
        registry()[name] = reinterpret_cast<void*>(func);
        return true;
    }
};

class Base : public myFactory {
   public:
    virtual void f() = 0;
    virtual ~Base() = default;
};

class DerivedExample : public Base {
   public:
    DerivedExample(int a, int b) { std::cout << a << " " << b << std::endl; }
    DerivedExample() { std::cout << "default" << std::endl; };

    static std::shared_ptr<Base> create() {
        return std::make_shared<DerivedExample>();
    }
    static std::shared_ptr<Base> create(int a, int b) {
        return std::make_shared<DerivedExample>(a, b);
    }

    virtual void f() override { std::cout << "DerivedExample" << std::endl; }

   private:
    // using one bool per function to register
    static bool sign1;
    static bool sign2;
    // properly getting the overloaded function addresses
    static std::shared_ptr<Base> (*pcreate1)();
    static std::shared_ptr<Base> (*pcreate2)(int, int);
};

std::shared_ptr<Base> (*DerivedExample::pcreate1)() = &DerivedExample::create;
std::shared_ptr<Base> (*DerivedExample::pcreate2)(int, int) =
    &DerivedExample::create;
// correctly passing the create function addresses
bool DerivedExample::sign1 =
    DerivedExample::sign("DerivedExample", DerivedExample::pcreate1);
bool DerivedExample::sign2 =
    DerivedExample::sign("DerivedExample2", DerivedExample::pcreate2);

int main() {
    std::shared_ptr<Base> p1 = Base::instantiate("DerivedExample");
    std::shared_ptr<Base> p2 = Base::instantiate("DerivedExample2", 1, 2);
    p1->f();
    p2->f();
}

Live

But I'm not sure it's the best design. I would prefer to register only one for the inherited class (though the above proposal shows that it is possible to do otherwise). Besides, the variadic signature was wrong, I fixed it by replacing forwarding-references by pass-by-value but it's not necessarily what you wanted to do.

[EDIT] Here is a solution without variadic and with a single registration, following my advice in comment.

#include <iostream>
#include <memory>
#include <string>
#include <unordered_map>

class Base;

class myFactory {
   public:
    typedef std::unordered_map<std::string, void *> registry_map;

    virtual ~myFactory() = default;

    static registry_map &registry() {
        static registry_map impl;
        return impl;
    }

    // here needing to pass by value in order to match your create function
    // signature
    template <typename... T>
    static std::shared_ptr<Base> instantiate(std::string const &name,
                                             T... args) {
        auto it = registry().find(name);
        if (it == registry().end()) return 0;
        typedef std::shared_ptr<Base> (*create_type)(T...);
        auto create_fun = reinterpret_cast<create_type>(it->second);
        return create_fun(args...);
    }

    template <typename F>
    static bool reg(const std::string &name, F func) {
        registry()[name] = reinterpret_cast<void *>(func);
        return true;
    }
};

class Base : public myFactory {
   public:
    virtual void f() = 0;
    virtual ~Base() = default;
};

// abstract base parameter class
struct Parameter {
    virtual ~Parameter() = 0;
};
Parameter::~Parameter() = default;

// parameter for constructor from void
struct VoidParameter : public Parameter {};

// parameter for constructor from two ints
struct TwoIntParameter : public Parameter {
    int a;
    int b;
    TwoIntParameter(int first, int second) : a(first), b(second) {}
};

class DerivedExample : public Base {
   public:
    DerivedExample(int a, int b) { std::cout << a << " " << b << std::endl; }
    DerivedExample() { std::cout << "default" << std::endl; };

    static std::shared_ptr<Base> create(const Parameter *param) {
        if (dynamic_cast<const VoidParameter *>(param)) {
            return std::make_shared<DerivedExample>();
        }
        if (dynamic_cast<const TwoIntParameter *>(param)) {
            TwoIntParameter const *ptr =
                dynamic_cast<const TwoIntParameter *>(param);
            return std::make_shared<DerivedExample>(ptr->a, ptr->b);
        }
        return nullptr;
    }

    virtual void f() override { std::cout << "DerivedExample" << std::endl; }

   private:
    // using one bool per function to register
    static bool sign;
};

// correctly passing the create function addresses
bool DerivedExample::sign =
    DerivedExample::reg("DerivedExample", &DerivedExample::create);

int main() {
    VoidParameter v;
    TwoIntParameter ti{1, 2};
    std::shared_ptr<Base> p1 = Base::instantiate("DerivedExample", &v);
    std::shared_ptr<Base> p2 = Base::instantiate("DerivedExample", &ti);
    p1->f();
    p2->f();
}

Live

You'll have to create a derived parameter type per constructor you want to support. You may avoid repetition according to what you exactly want to achieve.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.