3
\$\begingroup\$

This is an inversion-of-control object store. It is meant to be for a multithreaded application where it can load components at start-up based on complex environmental setup,

Worker threads can retrieve individual components on demand.

Do I need 2 overloaded functions of registerInstance()?

#include <iostream>
#include <vector>
#include <memory>
#include <ranges>
#include <memory>
#include <algorithm>
#include <unordered_map>
#include <iterator>
#include <any>
#include <typeindex>
#include <functional>

template <typename... Args>
concept NP = sizeof...(Args) == 1;
template <typename... Args>
concept PRS = sizeof...(Args) > 1;

class Factory
{
public:
    template <PRS T, typename... Ps>
    using Generator2 = std::function<std::unique_ptr<T>(Ps &&...arg)>;
    template <NP T>
    using Generator = std::function<std::unique_ptr<T>()>;

    template <NP T>
    void registerInstance(Generator<T> gen)
    {
        factoryMap_[typeid(T)] = gen;
    }
    template <PRS T>
    void registerInstance(Generator2<T> gen)
    {
        factoryMap_[typeid(T)] = gen;
    }
    template <NP T>
    std::unique_ptr<T> resolve()
    {
        auto it = factoryMap_.find(typeid(T));
        try
        {
            return it == factoryMap_.end() ? nullptr : std::any_cast<Generator<T>>(it->second)();
        }
        catch (const std::bad_any_cast &o)
        {
            // logit
            throw o;
        }
    }
    static Factory &getInstance()
    {
        static Factory instance;
        return instance;
    }

private:
    std::unordered_map<std::type_index, std::any> factoryMap_;
};
class DBOperations
{
public:
    virtual ~DBOperations() = default;
    virtual std::string handle() const
    {
        return "DB Opeation";
    }
};

class NOSQLOperations
{
public:
    virtual ~NOSQLOperations() = default;
    NOSQLOperations(std::unique_ptr<DBOperations> &&obj) : compD_{std::move(obj)} {}
    std::string handle() const
    {
        return compD_->handle() + "-NO SQL";
    }

private:
    std::unique_ptr<DBOperations> compD_;
};
class JsonParser
{
public:
    JsonParser(std::unique_ptr<NOSQLOperations> &&obj) : compC_{std::move(obj)} {}
    virtual ~JsonParser() = default;
    std::string handle() const
    {
        return compC_->handle() + "-Json Parser";
    }

private:
    std::unique_ptr<NOSQLOperations> compC_;
};
class Handler
{
public:
    virtual ~Handler() = default;
    Handler(std::unique_ptr<JsonParser> &&obj) : compB_{std::move(obj)} {}
    std::string handle() const
    {
        return compB_->handle() + "-Handler";
    }

private:
    std::unique_ptr<JsonParser> compB_;
};
void registerComponents()
{
    auto &ioc = Factory::getInstance();
    ioc.registerInstance<Handler>([&] { return std::make_unique<Handler>(ioc.resolve<JsonParser>()); });
    ioc.registerInstance<JsonParser>([&] { return std::make_unique<JsonParser>(ioc.resolve<NOSQLOperations>()); });
    ioc.registerInstance<NOSQLOperations>([&] { return std::make_unique<NOSQLOperations>(ioc.resolve<DBOperations>()); });
    ioc.registerInstance<DBOperations>([&] { return std::make_unique<DBOperations>(); });
}
int main()
{
    registerComponents(); // can be called at startup
    auto &ioc = Factory::getInstance();
    auto i = ioc.resolve<Handler>();
    std::cout << i->handle() << '\n';
}
\$\endgroup\$

1 Answer 1

Reset to default
1
\$\begingroup\$

Naming things

What are NP and PRS abbreviations of? I don't have a clue. Especially for concepts I would choose clear names that express what the concepts mean.

Factory is a very generic name. A factory of what? What if you have a code base which needs more than one factory type? I would choose a more distinctive name for this class, even if it is never visible to any other source files.

It's not thread safe

It is meant to be for a multithreaded application [...]

I don't see any mutexes or atomic operations in your code. Note that std::unique_ptr does not have anything to do with thread-safety.

No need for two overloads

Do I need 2 overloaded functions of registerInstance()?

No. Parameter packs can bind to zero or more arguments, so you don't need to make a special case for it. You can just write:

template <typename T, typename... Ps>
using Generator = std::function<std::unique_ptr<T>(Ps &&...arg)>;

template <typename T>
static void registerInstance(Generator<T> gen)
{
    getInstance().factoryMap_[typeid(T)] = gen;
}

What about multiple instances of the same type?

What if you have some complicated scenario where you have two different databases? Maybe you have two different classes derived from DBOperations, or maybe just the same class but you have to pass a different database URI to the constructor? Consider:

registerInstance<DBOperations>([] {
    return std::make_unique<MongoDBOperations>("example.com/db1");
});
registerInstance<DBOperations>([] {
    return std::make_unique<CouchDBOperations>("example.com/db2");
});

This is a problem, because there is only place for one DBOperations in your factoryMap_. Also problematic is that the second call to registerInstance() will silently overwrite the previous one.

Do you need a registry at all?

The problem with your registry is that it is indexed based on types. Since types need to be known at compile time, there is not much point in having a run time map. The only thing you want is to be able to write code that uses a given object before it is defined. But you can declare variables before assigning values, so you can do:

template <typename T, typename... Ps>
using Generator = std::function<std::unique_ptr<T>(Ps &&...arg)>;

Generator<Handler>         handlerGenerator;
Generator<JsonParser>      jsonParserGenerator;
Generator<NOSQLOperations> noSQLOperationsGenerator;
Generator<DBOperations>    dbOperationsGenerator;

handlerGenerator =         [&] { return std::make_unique<Handler>(jsonParserGenerator()); };
jsonParserGenerator =      [&] { return std::make_unique<JsonParser>(noSQLOperationsGenerator()); };
noSQLOperationsGenerator = [&] { return std::make_unique<NOSQLOperations>(dbOperationsGenerator()); };
dbOperationsGenerator =    [&] { return std::make_unique<DBOperations>(); };

auto handler = handlerGenerator();
std::cout << handler->handle() << '\n';

If you want to declare the instances in a header file that is included in multiple source files, you can make them inline.

Note that now you can easily declare two different generators of the same type.

\$\endgroup\$
3
  • \$\begingroup\$ Thank you for your review. Np => NoParams PRs=>Params I know it is not a thread safe, the registering happens at start-up before stating the child threads, but if new configuration is added at run time using any state command, , than I agree I would need a synchronizations, when I remove one instance of registerInstance the code doesn't compile? Have you tried it? no need to register all the instances, just an example. What about multiple instances of the same type? They would override each other, if needed can be addressed \$\endgroup\$ Commented Jan 16, 2023 at 20:55
  • \$\begingroup\$ Yes, using just one registerInstance() works for me, with both Clang and GCC. Make sure you also have just one Generator. \$\endgroup\$ Commented Jan 16, 2023 at 21:09
  • \$\begingroup\$ Yes, it worked and I removed the concepts as well. I like your question" Do you need a registry at all?" I think it can be written as you said, thx again for your time @G. Sliepen \$\endgroup\$ Commented Jan 16, 2023 at 21:28

You must log in to answer this question.