
Hi Vladimir, My congratulations on the success of Boost.Convert. But I can see you will not have much time to celebrate it, next library waiting in the queue. I wanted to share some of my thoughts on Boost.Pimpl library. This is by no means a review. I am not sure if I will have time to do a decent one. I only skipped through the documentation so my understanding is superficial, but I noticed some things that raise my immediate objection. Boost.Pimpl has been around for a while now, and I remember I was using it at some point in my work, although it didn't have the nice documentation format at that time. [1] The main point in using pimpl idiom (at least my) is to avoid header inclusion, which may grow compilation time too much (at the expense of certain run-time cost). But in order to use Boost.Pimpl I do need to include a header file, which in turn includes more header files. This is arguably less headers that in normal non-pimpl implementation, but it is more than if I used a raw pointer. And this is what I finally did in my work: switched to raw pointers for implementing piml in order to get the build perform faster. (I didn't measure the improvement though.) [2] Boost.Pimpl is good for everyone: people who like value semantics get the value semantic interface, and people who like shated_ptr's get shared_ptr semantics. Call me selfish or biased, but I believe using shared_ptr's should be banned (at least in 90% of the cases where it is currently being used), and the fact that you advertise it first in the docs makes me uneasy. It looks from the docs that 'pointer_semantics' implements something like a flyweight. This is so against value semantics. One of the expectations of types returned by value is that changing the original does no affect the copy: T a = c; T b = c; zap(a); assert(b==c && a!=b); Although the author of class Book knows that it is implemented with pointer_semantics, the users of the class are likely not to know it, and since Book doesn't look like a pointer, they will be expecting value semantics, and will be severely punished. [3] The docs advertize the definition of operator== as a member function. This is against a typical recommendation that it should be a free function in order to avoid asymmetrical behavior in certain cases. Does your library allow the definition of a non-member operator==? If so, I would encourage you to reflect that in the docs. [4] This page: http://yet-another-user.github.io/boost.pimpl/the_boost_pimpl_library/pimpl_... says "The advantage of the 'built-in' *Book::null()* [...] is that it makes possible an implementation of strongly exception-safe constructors, the copy constructor" There seems to be something wrong with this statement. How can a strong (commit-or-rollback) guarantee apply to constructors? If an exception is thrown from constructor the object is never created or accessible, so no-one cares about its state (as long as it doesn't leak). Or did you mean the no-throw guarantee? But the latter looks like an anti pattern. What is the value in concealing the exception (and information it carries) and instead produce null objects, which cannot be used? While I find the value_semantics interface useful, I am strongly against even offering the pointer_semantics interface. I am not sure if I have succeeded in explaining why. Regards, &rzej