Re: [boost] [review] Formal Fast-Track review of the Boost.Functional/Factory library - extended until sunday (on Behalf of Detlef Wilkening)

From: Tobias Schwinger (tschwinger_at_[hidden])
Date: 2007-12-27 11:19:58

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