2011/5/27 Sergiu Dotenco <sergiu.dotenco@gmail.com>
Am 26.05.2011 23:30, schrieb Krzysztof Czainski:
Looks like auto_ptr really isn't suitable for pimpl. And auto_ptr has many more draw-backs, so it is deprecated.
But getting back to the mini-review, suppose we use scoped_ptr instead of auto_ptr. I think Artyoms arguments still stand. And then you can't forget to make a non-inline destructor, because scoped_ptr uses checked_delete.
I certainly prefer using a specialized and well-tested library requiring just a couple of lines over manually implementing such an essential idiom by introducing boilerplate-code. Not mentioning the case when value semantics are wanted which scoped_ptr doesn't support.
Yes, I agree that scoped_ptr does not suffice, but I take Artyoms point about prefering a simple scoped_ptr (or as he said, auto_ptr) over the proposed Pimpl. Personally I think the proposed Pimpl library tries to solve too many things at once. Individual use cases might want those, not care about them, or specifically not want them. For example some class may want: relational operators; the proper deployment of the incomplete-type management technique (for the destructors to always work correctly). That class might not care about: support for in-place and replace boost::serialization; support for value and pointer semantics (and others if needed policies); support for the Bridge pattern; support for Non-Virtual Interface Idiom; And Pimpl providing the following: safebool operator; pimpl::null; can be an unwanted side-effect, and may therefore lead to a decision not to use the proposed Pimpl in this case. Many of the features addressed by Pimpl can be provided by separate tools, instead of packing all of them to Pimpl. That is why I'd be happy to see in boost: clone/copy_ptr, as well as a smart pointer with value semantics (I believe Artyom called it hold_ptr, and I like to call it value_ptr). These would also provide the "proper deployment of the incomplete-type management technique (for the destructors to always work correctly)". Relational operators can be easily provided using Boost.Operators. A subset of the operators would have to be implemented by hand, but putting them into Pimpl means every user of Pimpl get's them. And if that user doesn't want his class to have these operators? Will tools like the new traits for operators (has_operator_less etc.) detect them, when they shouldn't? As for safebool operator, I'd propose adding something like a boolable class to the operators library. It could require operator!, and implement a safebool operator in terms of op!. To conclude, I agree, that the problems addressed by the proposed Pimpl need solutions, but I suggest providing separate tools addressing the problems instead of Pimpl addressing all of these. With the proposed Pimpl, I would often choose not to use it and to implement pimpl by hand instead, just because Pimpl provides provides too many features. Regards Kris