
After almost two months I've finally uploaded an updated version.
Cliff wrote:
1. The Motivation and Rationale sections of the documentation needs to be much more detailed, comprehensive, and clear before I will use this library. To me, this is crucial for this library, since there must be clear advantages over existing practices and techniques (I think this is true of most "idiom" based libraries, versus libraries fulfilling a specific functionality need). I've tried hard to meet these requirements in the update.
-- Contrast your library with using boost::shared_ptr or boost::scoped_ptr - in particular, both shared and scoped ptrs support incomplete type usage (in the header). Right now, the only advantage I see in using pimpl_ptr versus shared / scoped ptr is in the policy behavior - lazy creation, etc. Will this advantage go away once/if a policy ptr is accepted into Boost? I've included an example called 'compare_to_other.cpp' which should show the advantages compared to other smart pointers. Should a policy_ptr be introduced to boost, I'll have to revise my work.
3. A documentation re-write for improved grammar and writing style is needed. I'm from Denmark, and English isn't my first language. Still I'm sorry if my english is really that bad :(
4. "In general" doesn't seem precise for the type requirements. Either type T must be copy constructible or not, or specify the requirements by pimpl_ptr methods / use. The tests should enforce the requirements. Also, looking at the code, there's at least a couple of areas where assignment is required for the contained type T (e.g. pimpl_ptr::operator=) and "operator==" is required for the contained type in at least two places. I'm not sure what you mean here. The examples given should very clearly show what is required from the pImpl class.
5. Looking at the code, the pimpl_ptr copy constructor is performing assignment in the ctor body ... how is the deep copy being performed? I don't see deep copy semantics in either the "base" class or the policy classes. Does this affect the requirements of the contained type T? Deep copying is performed by converting the pImpl pointer to a reference, and copying.
6. If a pimpl_ptr is default constructed with a manual_creation policy, how is the impl object created / passed in at a later time? By using 'set'.
7. One of the issues with policy based template classes is compatibility / conversion between types with different policies. In this library, pimpl_ptr<Foo> and pimpl_ptr<Foo, lazy_creation> are different types, and it's not clear to me how / if they can interact with each other. Since all policies implement the 'get' function, all relevant operations can be performed.
8. Swap is not swapping policies, only the underlying impl pointer (unless I'm confused). That's correct. You cannot swap policies. That's how C++ works (unless we recreate each pimpl_ptr, etc. but that would require more work than perhaps desired by the user).
Hope this helps, It helped a lot. Thank you.
Regards, Asger