I've uploaded another version with your suggestions. Thanks.
Rob Stewart wrote
----------------
operator=() does shallow copy of dynamically allocated value. Is this really what ypu want?
It's corrected.
I just downloaded the library and it still does a shallow copy. If you intend something else, then you need to rethink how you handle copying.
I think you should have a boost::pimpls namespace. In that you can have the pimpl class and the supplied policies. I renamed the policies namespace to pimpls. I keep the class itself inside
Could you be more specific? I really don't see where it shallows it. the boost namespace to shorten the name, eg. boost:pimpl instead of boost::pimpls::pimpl.
* The base class should be in namespace detail. Corrected.
I suggest a new type, derived from base, called, say, "default_construction" that can be the default policy. Corrected.
* Why is "PimplType" not just "T?" Corrected.
* base::get() error handling should be configurable or nothing. I've removed the exception, and rely on the user to know how to fix a null pointer.
* You m_pPimpl data member name is quite odd. Renamed to m_pImpl
* boost::pimpl_policies::base::base() misuses the new operator. corrected.
* boost::pimpl_policies::base::~base() should be non-virtual. Corrected.
* I'm not sure whether manual_creation should be a separate policy.
Why not have the default policy have a constructor taking a raw pointer, defaulted to a null pointer, plus provide a set() member function? You'd need to implement set() just as it is in manual_creation now.
namespace boost { namespace pimpls { template <typename T> struct default_ : base<T> { default_(T * pimpl_i = 0) : pimpl_(pimpl_i) { } T & get() const { return *pimpl_; } void set(T * pimpl_i) { delete pimpl_; pimpl_ = pimpl_i; } }; } }
Thus, default_ does what your base and manual_creation policies do in one simple policy.
But then you remove the default creation which is one of the advantages of the library.
* Don't define empty destructors.
They are noise in the code and can actually be a hindrance to optimal code generation. Corrected.
* pimpl's member functions should be inlined.
They are all trivial, so you should make them inline. Aren't templates always inlined?
* Consider "rhs" or "other" instead of "comparer." Corrected. I choose rhs.
Thanks for all the suggestions, Asger Mangaard