
From: "Asger Mangaard" <tmb@tmbproductions.com>
Rob Stewart wrote
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.
Could you be more specific? I really don't see where it shallows it.
Oops. I looked again. You're calling get() and that returns a reference. I had std::auto_ptr<T>::get(), for example, in mind when I saw that. You're fine, although *this = rhs is unusual for a copy constructor.
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 the boost namespace to shorten the name, eg. boost:pimpl instead of boost::pimpls::pimpl.
If someone wants it shortened, they can use a using declaration. I'm not sure this warrants being directly in the boost namespace.
* 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.
But then you remove the default creation which is one of the advantages of the library.
Ummm, yeah. Sorry. I was only paying attention to one side of that coin, it seems. It occurs to me that the creation policy names could be shortened to "lazy," "manual," and "default_."
* pimpl's member functions should be inlined.
They are all trivial, so you should make them inline. Aren't templates always inlined?
Nope. They work just like non-templated (member) functions. -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;