[review] Formal Fast-Track review of the Boost.Functional/Factory library begins today
Hi all, Today starts the formal Fast-Track review of the Boost.Functional/Factory library. *Description* Factories are callback mechanisms for constructors, so we provide two function objects, boost::value_factory and boost::factory, that encasulate object construction through direct application of the constructor or operator new, respectively. These templates make other facilities that operate on function objects (such as standard algorithms, Boost.Bind, Boost.Function, or Fusion functional adapters) applicable to constructors. *Author* Tobias Schwinger *Download* Get it from here: http://www.boost-consulting.com/vault/index.php?action=downloadfile&filename=factory.zip&directory=X-Files& Read documentation online here: http://torjo.com/tobias/ What to include in Review Comments ================================== Your comments may be brief or lengthy, but basically the Review Manager needs your evaluation of the library. If you identify problems along the way, please note if they are minor, serious, or showstoppers. Here are some questions you might want to answer in your review: * What is your evaluation of the design? * What is your evaluation of the implementation? * What is your evaluation of the documentation? * What is your evaluation of the potential usefulness of the library? * Did you try to use the library? With what compiler? Did you have any problems? * How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? * Are you knowledgeable about the problem domain? And finally, every review should answer this question: * Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion. Best, John Torjo - Review Manager - -- http://John.Torjo.com -- C++ expert ... call me only if you want things done right
On 12/17/07 04:22, John Torjo wrote:
Hi all,
Today starts the formal Fast-Track review of the Boost.Functional/Factory library.
[snip]
What to include in Review Comments ==================================
[snip]
* What is your evaluation of the documentation?
Good, however, it should include some hint about how to overcome the Lvalue constraint on the arguments that's mentioned in the docs: a0...aN argument LValues to a constructor of T I'd think something like the following: template < class Source
struct factory_fwd : boost::forward_adapter<boost::factory<Source> > { factory_fwd(void) : boost::forward_adapter<boost::factory<Source>
(boost::factory<Source>()) {} };
would nicely show how the forward lib can be used to overcome this constraint.
* What is your evaluation of the potential usefulness of the library?
Very. I'm thinking it could be used as a replacement for Abraham's new_ as shown in the attachment to: http://lists.boost.org/Archives/boost/2007/12/131251.php I think it could also be used for similar factories which, for example, want to decorate the source object with some hidden features, e.g. a refcount. The containing smart_ptr would know about that and somehow retrieve it and avoid the extra overhead of a detached refcount as in, for example, shared_ptr.
* Did you try to use the library? With what compiler? Did you have any problems?
Compiled factory.cpp and factory_fwd template with gcc-4.1 and had no problems.
* How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
Quick reading.
* Are you knowledgeable about the problem domain?
Not very.
And finally, every review should answer this question:
* Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion.
Yes.
Hello Larry, thank you for your positive review. Larry Evans wrote:
On 12/17/07 04:22, John Torjo wrote:
Hi all,
Today starts the formal Fast-Track review of the Boost.Functional/Factory library.
[snip]
What to include in Review Comments ==================================
[snip]
* What is your evaluation of the documentation?
Good, however, it should include some hint about how to overcome the Lvalue constraint on the arguments that's mentioned in the docs:
Yes good point. It should mention there's Functional/Forward (I think I left it out because I didn't know whether Forward was going to be accepted into Boost).
* What is your evaluation of the potential usefulness of the library?
Very. I'm thinking it could be used as a replacement for Abraham's new_ as shown in the attachment to:
http://lists.boost.org/Archives/boost/2007/12/131251.php
I think it could also be used for similar factories which, for example, want to decorate the source object with some hidden features, e.g. a refcount. The containing smart_ptr would know about that and somehow retrieve it and avoid the extra overhead of a detached refcount as in, for example, shared_ptr.
I'm not sure that I completely understand your suggestion. However it sounds like something that's probably implemented by (or at least easily implementable with) Joaquin M. Lopez' Flyweight library. In case you don't need state in the factory itself, you might get away with something like map< string, function< smart_ptr1<T>(args) > > factories; // ... factories["foo"] = factory< smart_ptr2<U> >(); factories["bar"] = factory< smart_ptr2<V> >(); given conversion sequences smart_ptr1<U> --> smart_ptr2<U> smart_ptr1<V> --> smart_ptr2<V> . Regards, Tobias
This is not a full review, just a few comments: The supplied documentation mentions nothing about the lifetime of a factory. There are two features that I would need from a factory system like this. First, the ability to register and unregister factories. 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. 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. 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. -- Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode On Dec 17, 2007 2:22 AM, John Torjo <john.groups@torjo.com> wrote:
Hi all,
Today starts the formal Fast-Track review of the Boost.Functional/Factory library.
*Description*
Factories are callback mechanisms for constructors, so we provide two function objects, boost::value_factory and boost::factory, that encasulate object construction through direct application of the constructor or operator new, respectively. These templates make other facilities that operate on function objects (such as standard algorithms, Boost.Bind, Boost.Function, or Fusion functional adapters) applicable to constructors.
*Author*
Tobias Schwinger
*Download*
Get it from here: http://www.boost-consulting.com/vault/index.php?action=downloadfile&filename=factory.zip&directory=X-Files&
Read documentation online here: http://torjo.com/tobias/
What to include in Review Comments ==================================
Your comments may be brief or lengthy, but basically the Review Manager needs your evaluation of the library. If you identify problems along the way, please note if they are minor, serious, or showstoppers.
Here are some questions you might want to answer in your review:
* What is your evaluation of the design? * What is your evaluation of the implementation? * What is your evaluation of the documentation? * What is your evaluation of the potential usefulness of the library? * Did you try to use the library? With what compiler? Did you have any problems? * How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? * Are you knowledgeable about the problem domain?
And finally, every review should answer this question:
* Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion.
Best, John Torjo - Review Manager -
-- http://John.Torjo.com -- C++ expert ... call me only if you want things done right
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
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
participants (4)
-
Emil Dotchevski -
John Torjo -
Larry Evans -
Tobias Schwinger