Re: [boost] [review][pimpl_ptr] Pimpl Pointer review begins

From: Anthony Williams (anthony_w.geo_at_[hidden])
Date: 2006-05-15 06:45:02

Summary: I vote NO to including pimpl_ptr in boost.

There is a better implementation (entitled grin_ptr) available from Alan
Griffiths' website (http://www.octopull.demon.co.uk/c++-writing.html). Alan
describes a new revision of his code in the April 2006 issue of Overload
(http://accu.org/index.php/aboutus/latestjournals), but I don't think this is
on his website as yet.

Rene Rivera <grafik.list_at_[hidden]> writes:

> - Are you knowledgeable about the problem domain?

Yes. I've written smart pointers before, and used the pimpl idiom. I am aware
of the issues that surround such usage.

> - What is your evaluation of the design?

The policy based design is overly complex. The lazy initialization feature
is really orthogonal to the use of the pimpl idiom.

> - What is your evaluation of the implementation?

The current implementation requires that the implementation type T be complete
for the copy constructor, assignment operator and destructor, in order to
perform the deep copies. Therefore classes that use pimpl will need to provide
out-of-line definitions of these members, even if they are empty (which is the
expected case).

> - What is your evaluation of the documentation?

The documentation seems to clearly explain the intended usage, though there
are a few typos (e.g. inconsistent case in the class names/constructor names
in the example).

> - What is your evaluation of the potential usefulness of the library?

Reasonable. Pimpl is a relatively common idiom, and it would be good to have a
boost library that addressed the appropriate issues.

> - Did you try to use the library? With what compiler?

Yes. g++ 4.0.1

>    Did you have any problems?

Yes.

pimpl_ptr.hpp line 129 needs getNew() to be qualified with this->, since it is
a non-dependent name, but it is defined in a dependent base class.

As mentioned above, the implementation doesn't match the docs, and the
gameplayer example from the docs doesn't compile, as the compiler-generated
members of CGamePlayer need CGamePlayerValues to be a complete type.

> - How much effort did you put into your evaluation? A glance? A
>    quick reading? In-depth study?

Reasonably in-depth study of code. Quick glance through docs.

Anthony
-- 
Anthony Williams
Software Developer
Just Software Solutions Ltd
http://www.justsoftwaresolutions.co.uk