
Hello Emil, thanks for your comments. Emil Dotchevski wrote:
This is not a full review, just a few comments:
The supplied documentation mentions nothing about the lifetime of a factory.
A factory (in this context) is just a function object.
There are two features that I would need from a factory system like this.
First, the ability to register and unregister factories.
A registry is intentionally not provided as the requirements typically differ greatly between projects. Although this submission makes it trivial to write one from scratch, map< string, boost::function< favorite_smart_ptr<T>(args) > > to begin with.
I would suggest explicit registration that returns a shared_ptr to the factory function, while keeping weak_ptr to it. This way, if the factory serves a type that is defined in a DLL, it can be automatically unregistered when the DLL is unloaded.
Something like this // We're in DLL.CPP here namespace { shared_ptr< function< plugin*() > > sentinel; } void plugin_entrypoint(plugin_registry r) { sentinel.reset(r.register_plugin( "my_plugin", factory<my_plugin*>())); } // Also note: The code is untested and unpolished - // it's provided for communication purposes only. ? Now that I look at some code it seems like it might work but it's certainly not the be-all end-all of factory registries, 'plugin_registry' is still a trivial thing and returning that 'shared_ptr' seems sorta weird (though it admittedly saves us some work). Further, automatic unloading won't work portably with some ABIs.
Second, I would prefer the factory table to not be global, instead its type can be documented so it is up to the client code to decide if they want it to be global or not.
If I wanted to provide a registry, I'd certainly agree here.
Finally, it's worth mentioning that in the presence of DLLs using type_info directly is unsafe, since dereferencing a pointer that points a type_info object obtained from a DLL causes a crash if the DLL is unloaded.
Yeah, accessing unloaded memory (or even running unloaded code) seems a bad idea in general not only with 'type_info' objects ;-)... Regards, Tobias