[review] Formal Fast-Track review of the Boost.Functional/Factory library - 2 days left
Hi all, There are only 2 days left from the formal Fast-Track review of the Boost.Functional/Factory library. We've had just one review, and we need more. If you are interested in the library (you should be :)), please take some time to review it. This is the info you need to do a review on: *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
John Torjo wrote:
* What is your evaluation of the design?
"Factory" is a succinct shorthand for "constructor/new forwarding library", but I must admit that the name Factory implies more to me -- something more like the GoF Factory pattern. I used the library to build a toy version of a Factory mechanism from a previous project. With that, the client passes a std::string name and gets a pointer to the corresponding concrete implementation of an abstract base class. To be sure, the messiest part of my previous implementation was passing constructor arguments. It was nice that boost::factory eliminated the need to handle that explicitly.
* What is your evaluation of the implementation?
I didn't look at the implementation.
* What is your evaluation of the documentation?
It could be more tangible. As I read it, my question was: how can I use this library to build the functionality described above? To me, the most useful part of the documentation was the hint about boost::function and the examples. Several more examples would be great. I'd particularly like to see the boost::function trick illustrated in more depth, e.g.: here's a std::map<std::string, boost::function<Base*()> > which we can populate with boost::factory<SubclassOne*>(), boost::factory<SubclassTwo*>(), etc. Since (to me) the real power of boost::factory is that it forwards constructor arguments, extending that example to show a non-empty canonical constructor argument signature (e.g. a mapped_type of boost::function<Base*(int)>) would be good too. But lack of that extended documentation shouldn't be a showstopper.
* What is your evaluation of the potential usefulness of the library?
It's a very useful building block.
* Did you try to use the library? With what compiler? Did you have any problems?
I tried boost::factory with Apple's g++ 4.0.1. I had no problems. I didn't try boost::value_factory.
* Do you think the library should be accepted as a Boost library?
Yes. My one concern is that perhaps the name boost::factory should be reserved for a later, more full-featured library, built on this one -- though I haven't yet thought of a more descriptive name for this one that's both clear and concise.
Hello Nat, thanks for reviewing the Functional/Factory utility and for the positive vote. Nat Goodspeed wrote:
John Torjo wrote:
* What is your evaluation of the design?
"Factory" is a succinct shorthand for "constructor/new forwarding library", but I must admit that the name Factory implies more to me -- something more like the GoF Factory pattern.
Well, we have to keep in mind that the GoF book was written more than a decade ago. The abstract definitions of Design Patterns are pretty much timeless, however, the implementations in today's C++ can look different - even incomplete (details below).
I used the library to build a toy version of a Factory mechanism from a previous project. With that, the client passes a std::string name and gets a pointer to the corresponding concrete implementation of an abstract base class.
To be sure, the messiest part of my previous implementation was passing constructor arguments. It was nice that boost::factory eliminated the need to handle that explicitly.
* What is your evaluation of the implementation?
I didn't look at the implementation.
* What is your evaluation of the documentation?
It could be more tangible. As I read it, my question was: how can I use this library to build the functionality described above? To me, the most useful part of the documentation was the hint about boost::function and the examples.
If we want to create derived classes from an abstract base, boost::function needed. But is this isn't the only way to implement the Factory Patterns, is it? If we want to create just objects that fulfill a certain concept we don't need boost::function at all. Note that the Patterns' definitions still hold - it just isn't all spelled out; the concept is not formalized in the programming language and so the need for a Factory base class and the implicit upcast of the Product vanishes as well. So, it pretty much comes down to that OOD Patterns look different when they're applied at generic level. Also note we're not tied to class level granularity anymore, e.g: class some_class { // ... // Observer Pattern (all what's left of it): void register_observer(function<void()> observer_function); }; It's much nicer than the traditional GoF code, isn't it? We have both eliminated the coupling and the boilerplate code!
Several more examples would be great. I'd particularly like to see the boost::function trick illustrated in more depth, e.g.: here's a std::map<std::string, boost::function<Base*()> > which we can populate with boost::factory<SubclassOne*>(), boost::factory<SubclassTwo*>(), etc.
Since (to me) the real power of boost::factory is that it forwards constructor arguments, extending that example to show a non-empty canonical constructor argument signature (e.g. a mapped_type of boost::function<Base*(int)>) would be good too.
But lack of that extended documentation shouldn't be a showstopper.
* What is your evaluation of the potential usefulness of the library?
It's a very useful building block.
* Did you try to use the library? With what compiler? Did you have any problems?
I tried boost::factory with Apple's g++ 4.0.1. I had no problems. I didn't try boost::value_factory.
* Do you think the library should be accepted as a Boost library?
Yes. My one concern is that perhaps the name boost::factory should be reserved for a later, more full-featured library, built on this one -- though I haven't yet thought of a more descriptive name for this one that's both clear and concise.
My personal feeling towards a more full-featured 'boost::factory' is that it will be either overly simplified or a tool to break a butterfly on a policy-based wheel... Regards, Tobias
Tobias Schwinger wrote:
We have to keep in mind that the GoF book was written more than a decade ago. The abstract definitions of Design Patterns are pretty much timeless, however, the implementations in today's C++ can look different - even incomplete (details below).
I used the library to build a toy version of a Factory mechanism from a previous project. With that, the client passes a std::string name and gets a pointer to the corresponding concrete implementation of an abstract base class.
If we want to create derived classes from an abstract base, boost::function needed. But is this isn't the only way to implement the Factory Patterns, is it?
If we want to create just objects that fulfill a certain concept we don't need boost::function at all. Note that the Patterns' definitions still hold - it just isn't all spelled out; the concept is not formalized in the programming language and so the need for a Factory base class and the implicit upcast of the Product vanishes as well.
So, it pretty much comes down to that OOD Patterns look different when they're applied at generic level. Also note we're not tied to class level granularity anymore, e.g:
class some_class { // ...
// Observer Pattern (all what's left of it): void register_observer(function<void()> observer_function); };
It's much nicer than the traditional GoF code, isn't it? We have both eliminated the coupling and the boilerplate code!
I'm willing to concede that I may be burdened with outdated notions of how to solve such problems. I would be very interested to know how I should do it better. The issue I mentioned above keeps recurring across different projects: given a string from a file or a script, I need to instantiate a distinct C++ class corresponding to that string. I mention an abstract base class because I don't know how, other than virtual functions, I should access the features of such an object. I would dearly love to see the best way to leverage boost::factory to address this problem. If, in the process, I learn new tricks myself, all the better! Perhaps the library is more squarely targeting a whole family of real-world problems that I haven't yet encountered. I wouldn't mind being educated about those use cases, either. You see where I'm going with this. More examples would really help -- not just when trying to use the library, but even to decide whether this library can help me solve the problem at hand.
My one concern is that perhaps the name boost::factory should be reserved for a later, more full-featured library, built on this one -- though I haven't yet thought of a more descriptive name for this one that's both clear and concise.
My personal feeling towards a more full-featured 'boost::factory' is that it will be either overly simplified or a tool to break a butterfly on a policy-based wheel...
Heh. I hope my remarks about the name boost::factory didn't come across as arrogant or condescending. If so, profuse apologies! It may well be that the library, just as it stands, is far more powerful than I have yet realized. Again, I'd love to be enlightened about other use cases. Thanks.
Nat Goodspeed:
The issue I mentioned above keeps recurring across different projects: given a string from a file or a script, I need to instantiate a distinct C++ class corresponding to that string. I mention an abstract base class because I don't know how, other than virtual functions, I should access the features of such an object.
I would dearly love to see the best way to leverage boost::factory to address this problem. If, in the process, I learn new tricks myself, all the better!
Typically you'll have something like std::map< std::string, boost::function<std::auto_ptr<A>()> > s_factory; where A is your abstract base, and initialize s_factory with s_factory[ "X" ] = boost::factory< std::auto_ptr<X> >();
Peter Dimov wrote:
given a string from a file or a script, I need to instantiate a distinct C++ class corresponding to that string.
Typically you'll have something like
std::map< std::string, boost::function<std::auto_ptr<A>()> > s_factory;
where A is your abstract base, and initialize s_factory with
s_factory[ "X" ] = boost::factory< std::auto_ptr<X> >();
Thanks. That resembles what I built myself. I'd still like to see something very like this in the documentation examples. I may still be missing opportunities for compile-time polymorphism, and perhaps it's true that including machinery like the above as part of an expanded Boost library would unnecessarily constrain the uses of the existing library. I'd like to see examples of some of those other uses, too.
Nat Goodspeed wrote:
I'm willing to concede that I may be burdened with outdated notions of how to solve such problems. I would be very interested to know how I should do it better.
That's really not my point! I didn't say (nor even think) your code was inferior -- even if it was, it probably would've been my fault providing insufficient documentation. ( I like being misunderstood as little as I like to misunderstand in this particular way... Please don't :-). ) What I was trying to say is that your use case (although common) is not the only one there is...
The issue I mentioned above keeps recurring across different projects: given a string from a file or a script, I need to instantiate a distinct C++ class corresponding to that string. I mention an abstract base class because I don't know how, other than virtual functions, I should access the features of such an object.
...and that it varies in nuances: In one project we'll want a search tree in the other one a hash table (size vs. speed). In one project we'll want to use 'typeid(T).name()' for the search key another one will need fixed integral IDs and yet another one will assign integral IDs at runtime and finally one project will use strings... The Factory utility should make those things sufficiently trivial to implement, however.
I would dearly love to see the best way to leverage boost::factory to address this problem. If, in the process, I learn new tricks myself, all the better!
Guessing from the follow-up you might have used the best way for your particular case. Now, if we can make everything a template and code towards an interface defined by a Concept instead of an abstract base class (I know it isn't always a good idea), we don't need Boost.Function to introduce runtime- polymorphism (there just is none) and we can shift the invocation of an abstract member function from the inside of a performance-critical code block to its invocation...
Perhaps the library is more squarely targeting a whole family of real-world problems that I haven't yet encountered. I wouldn't mind being educated about those use cases, either.
You see where I'm going with this. More examples would really help -- not just when trying to use the library, but even to decide whether this library can help me solve the problem at hand.
Yes, your critique is well-taken. The documentation can use some more detail in this place.
My personal feeling towards a more full-featured 'boost::factory' is that it will be either overly simplified or a tool to break a butterfly on a policy-based wheel...
Heh. I hope my remarks about the name boost::factory didn't come across as arrogant or condescending. If so, profuse apologies!
Just to make one thing clear: I'm not enough of a sadist to get high by pissing off others telling them how much they suck. When I do, it makes me feel sorry, but this time it's really just a misunderstanding ;-). So: All I was trying to say is that we probably either won't agree on a single best way to write 'boost::factory' or introduce so many Policies that usage becomes more complicated than a from-scratch implementation.
It may well be that the library, just as it stands, is far more powerful than I have yet realized. Again, I'd love to be enlightened about other use cases.
Its power stems from the combination with well-established Boost components and generic programming techniques. Regards, Tobias
participants (2)
-
John Torjo -
Nat Goodspeed -
Peter Dimov -
Tobias Schwinger