Andrzej, Thank you for your quick reply. :-) ... I did not even know Glen already advertised the upcoming review. Please find my replies below. (I snipped some for brevity. My apologies) V. On 06/06/2014 07:21 AM, Andrzej Krzemienski wrote:
...
[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.)
Well, I feel you mix up two things here: pre-build analysis and the build as such. The number of included headers has impact on the first phase -- pre-build analysis -- as 'make' has to check those included-files timestamps. I do not believe it's such an issue as I do not expect to notice much/any performance differences from checking 1 of 10 timestamps. So, if you have pimpl.hpp included with a few other Boost headers thrown into the mix, then 'yes' it does increase the time of pre-build analysis... which is negligible compared with the time of the actual build. Given those mentioned Boost headers do not change, they do not trigger re-build. That said, when re-build is triggered by other code, those headers to need to be compiled and that takes time. Now we are getting to the build itself. The thing is that build performs the fastest when it, well, does not build. And that's what Pimpl allows you to achieve. So, in short, if you deploy the Pimpl idiom, then you manage to avoid re-builds. The time/performance difference between your solution and mine will only be in pre-build-analysis time... which I claim to be fairly minimal if noticeable at all. As for doing Pimpl yourself vs. a library solution, then it's a different matter. I'll take the latter any day... and will make sure people on my project do the same... if they want their code to pass through review. :-)
[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.
We might be developing for different industries. I've been working for rail and air for the last 15 years and there shared data are everywhere. Rail network topology is only one, aircraft data, airline schedules are all shared by many threads. So, my focus on shared_ptr-based pimpl::pointer_semantics is clearly biased.
It looks from the docs that 'pointer_semantics' implements something like a flyweight.
Yes, it is a variation of the Flyweight idiom... as shared_ptr and raw pointers are... Although I personally do not like the name as I find it confusing. In GoF it is the shared object (big and expensive to copy) which is called "flyweight object". To it is the proxies (shared_ptr, raw pointers) which look like "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.
Well, I certainly agree that value-semantics classes are quite different from pointer-semantics classes... and it is expected that user knows what he/she is using. Pimpl tries to help by making it clear in the declaration: class Book : public pimpl<Book>::pointer_semantics class Book : public pimpl<Book>::value_semantics To me "doesn't look like a pointer" sounds like a weak argument because there are many things in C++ which do not look like a pointer: 1) non-const reference; 2) typedef shared_ptr<foo> my_foo; 3) many classes in X Window System (like Widget) are actually pointers. What I am driving at is that the user ultimately needs to know what class and its behavior actually are to use it effectively. That said, I do acknowledge a potential for mis-use. In my code I address it with "const". That is, say, shared_ptr<foo const> is passed around and has to be copied to shared_ptr<foo> before being modified.
[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.
op==() as a member works well for object of he same type, like pimpl1 == pimpl2... and for pimpls I believe it's all what's needed. I might be wrong though (just have not had practical applications of different sort). I'll look into it though.
[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?
I could not remember what I based those statements on. :-( I started researching and writing a reply but got totally tangled up with all those exception-guarantee complexities... Got totally confused... removed the part.
... While I find the value_semantics interface useful, I am strongly against even offering the pointer_semantics interface.
Hmm, that "strongly against" indicates some conviction and I think Gavin echoed that view in a later post. I'll probably need to revisit my lib designs as I am using it a lot. For example, right now I read a DB, cache it. Then everyone has read-only access to records via pimpl-like shared_ptr-based facility. It's all kosher as read/access requests mutexed and fast. Where is the problem? I seem to be missing something big.