Peter Dimov wrote:

Just a quick note, and sorry that I haven't reviewed the library yet:

When doing a *fast-track* review, it would be helpful to note which Boost components already use the library, and in what form.

Unlike the former, this one's not a factorization. That list under http://www.boost.org/more/formal_review_process.htm#Fast-Track isn't meant to be AND-reduced, is it? Anyway, it was the Wizard's suggestion (I originally asked for a single, collective review for all the tiny bits and pieces), so I figure it'll be OR. Regards, Tobias

Frank Mori Hess wrote:

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

On Friday 21 December 2007 07:33 am, John Torjo wrote:

...

Read documentation online here: http://torjo.com/tobias/

There seems to be a typo in the documentation for value_factory, in the notation where it says:

F the type value_factory<F>

I think it's supposed to be value_factory<T>

Yep. Thanks, Tobias

shunsuke wrote:

John Torjo wrote:

...

Hi all,

I've extended the review until Sunday (inclusive). I'm hoping for more reviews. So far, we've had only two. 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.

Is there any difference to lambda::constructor/new_ptr?

Yes, those components are designed to only work with the Lambda library. Functional/Factory OTOH provides standard compliant (they work with result_of and the libraries built upon it) function objects. Regards, Tobias

On Dec 21, 2007 12:31 PM, Stjepan Rajko <stipe@asu.edu> wrote:

Otherwise, I would jump with joy if all of these adapters used *protected* inheritance so I could also dig up parts of an inherited class's interface (say, counting_factory above could have a public

Hmm... I'm not sure that I know exactly what I'm talking about when insisting the inheritance should be *protected*... maybe just ignore it ... serves me right for bolding something like that :-) But this did make me think about whether using compressed_pair is the right thing to do, because maybe there are cases where things would be useful as inherited even when the base class is not empty. Stjepan

On 12/24/07, Jens Seidel <jensseidel@users.sf.net> wrote:

On Mon, Dec 24, 2007 at 02:41:06AM +0200, John Torjo wrote:

...

Hello John Torjo,

Oops, this mail confused me! It was probably not written by John, even if the FROM: line says so but from:

...

Greetings

Detlef Wilkening

Detlef, don't fake your mails!

It is likely that John Torjo received the email privately and forwarded it to the mailing list, and forgot to add a notice. gpd

Hello! John Torjo wrote:

Hello John Torjo,

I don't know where to send my review of Boost.Factory library - therefore i send it to you. I hope, that until my next review i know it better, but today i must answer before the time to review has gone.

...

* What is your evaluation of the design? * What is your evaluation of the implementation?

I like, that the lib has direct support for smart pointer, because this is a normal technic in C++.

But i often has the problem of a 2 phase initialisation. I don't like it, but i found it very often in practice. After the constructor for example i must call an init function. Or sometimes i must do things which are not possible in a constructor. I would like, if the factory would support such mechanism.

I would like a third (or better an second template argument, and the allocator become the third argument), which is a policy class for the two phase construction. After the new the factory call a static function in the policy class. Default policy is a class which does nothing. But here I can write a class, which calls e.g. a init function or other things.

Honestly, I think that RAII is almost always the right design. I believe that "two phase construction" is often just bad practice (at least if we can use exceptions). So if for some reason we can't get around using "two phase construction" it can (and should) be done in user code: The Factory utility is about turning a new expression or constructor into a functional - nothing more as beyond that thing are sufficiently trivial to implement (given other Boost tools, that is) and vary greatly on a per-use-case-basis. Once the constructor is run you can use a virtual function to invoke the second phase: class A { public: virtual void init() = 0; virtual void foo() = 0; // ... }; class B : public A { public: B() { cout << "B::B" << endl; } void init() { cout << "B::init" << endl; } void foo() { cout << "B:foo" << endl; } }; // ... map<string,function<A*()> > factory_registry; int main() { factory_registry["B"] = factory<B*>(); //... A* a1 = factory_registry["B"](); a1->init(); // ... delete a1; return 0; }

Second I would like a name like factory_ptr or factory_pointer more than only factory for the class. It shows more whats the problem domain of the factory, and stands more in parallel to factory_value.

I disagree - it's just confusing :-) as one might conclude the factory itself for being a smart pointer or being applicable only to pointers. A similar argumentation applies to "factory_value".

...

* What is your evaluation of the documentation?

I would like a better motivation. I have discuss the lib with a friend to understand why I need such a lib. I think, not everybody (I include) has a deep understanding of modern C++ technics, and therefore a better motivation with two more detailed examples would help the beginners.

Yes. I'll add two more complete examples. Thanks for your review! Regards, Tobias