[pimpl] Some comments

newer
[core] Breaking bugfix in scoped...

Andrzej Krzemienski

5 Jun 2014 5 Jun '14

9:21 p.m.

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

Show replies by date

Gavin Lambert

6 Jun 6 Jun

1:35 a.m.

On 6/06/2014 09:21, quoth Andrzej Krzemienski:

...

[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.)

As I see it, the main benefit of pimpl comes from avoiding recompiles due to *mutable* header files -- ie. if I make a change in private implementation of a class, I don't want to have to recompile all consumers of that class (but that's needed if I change the public interface). Depending on (mostly) immutable header files (such as Boost.Pimpl itself) is usually not a problem; you can resolve compilation-time issues with these by using precompiled headers.

...

[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.

+1.

...

[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?

I presume this is related to the Null Object Pattern, which TBH I've never been entirely comfortable with -- while at first glance it might make the code tidier to avoid explicit null checks and instead provide a reasonable do-nothing-return-defaults immutable fake/"null" object, the reality is that you often want completely different behaviour for null vs. non-null cases and it's usually not sensible to pretend the action was successful while actually doing nothing, so you end up putting the null checks back in anyway. And instead of a simple "is this pointer null" check which the compiler can almost always optimise down to almost nothing, you now have an "is this object equal to the null object" check, which can get quite heavy depending on how things are internally implemented.

Vladimir Batov

1:51 a.m.

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.

Rob Stewart

3:13 a.m.

On June 5, 2014 9:51:18 PM EDT, Vladimir Batov wrote:

...

...
...

[1] The main point in using pimpl idiom (at least my) is to avoid

...
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

On 06/06/2014 07:21 AM, Andrzej Krzemienski wrote: header 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.

You've glossed over the overhead of compiling your headers when the implementation details change. I doubt that adds a lot of time, but it isn't zero either.

...

...
[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.

I took a quick look at the docs. First, like Edward, I was frustrated by how much I had to read to understand what the library offered and how. Second, you've extended the Pimpl Idiom. It has always been about moving the implementation details out of the header, not sharing those details. Thus, your value semantics implement the idiom and your pointer semantics provide something different. If you don't try to shoehorn both into "Pimpl", you'll get better traction. I suggest "shared_impl" and "pimpl" as the names of the two class templates. ___ Rob (Sent from my portable computation engine)

Vladimir Batov

4:14 a.m.

On 06/06/2014 01:13 PM, Rob Stewart wrote:

...

... I took a quick look at the docs. First, like Edward, I was frustrated by how much I had to read to understand what the library offered and how. Second, you've extended the Pimpl Idiom. It has always been about moving the implementation details out of the header, not sharing those details. Thus, your value semantics implement the idiom and your pointer semantics provide something different.

... Hmm, that never occurred to me. I've never looked at my implementation as an extension of the pattern. I always felt I was true to the source so to speak. I have to think about it but my initial reaction is somewhat reluctance to accept that. Maybe I am too focused on or accustomed to the "pointer" part as "pointer" is all about sharing something cheaply. To me the first "implementation" of the Pimpl pattern was Widget in X Window System -- a hidden (unavailable in its entirety) implementation... only available via a handle/pointer... "pointer to implementation" does not get any more and purer than that... and obviously Widgets are passed around and the content they point to is shared. Hmm, I'll have to read more over the weekend.

Andrzej Krzemienski

7 Jun 7 Jun

12:05 p.m.

2014-06-06 3:51 GMT+02:00 Vladimir Batov :

...

Andrzej, Thank you for your quick reply. :-) ... I did not even know Glen already advertised the upcoming review.

This looks like a really good idea. A pre-review notice, which gives us an opportunity to clear up many thing before even the review is started.

...

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. :-)

You convince me. What you and others in this thread say indicate that I do not fully understand the benefit of pimpl idiom.

...

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.

While I agree with this last statement alone, ("you must know the types you use"), I am uneasy with the conclusion about pimpl::pointer_semantics. I realize that so far the only thing I expressed is my *preference*, which may be too little for a discussion, so I will try to come up with more objective arguments (and see if there are any, or if this is just a personal taste). I still claim that pimpl::pointer_semantics breaches value semantics expectations in an unprecedented way. Both non-const refs and shared_ptr, and raw pointer and normal value semantic classes have one property in common: Their operator== reflects their *salient attributes*. Sorry for a fancy term. I borrowed it from John Lakos: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2479.pdf, https://github.com/boostcon/cppnow_presentations_2014/blob/master/files/accu... Let me illustrate this. When I have two objects of type 'book` with the following members for accessing key components of the value Book book1 = make_book(); book1.title() book1.author(); I expect that expression (book1.title() == book2.title() && book1.author() == book2.author()) is equivalent to (book1 == book2). Note that this expectation holds for references. It also holds for smart (and dumb) pointers: std::shared_ptr<Book> book1 = make_book(); book1 does not have member title(). The following is invalid: book1.title(); // INVALID the indirection operator -> or * already tells me: these remote parts are not part of book1 value. The value of the pointer is the address it stores. It is consistent in the sense that its operator== only compares its value. In contrast, pimpl::value_semantics give me direct (no operator* or ->) access to title() and author(), and a surprising behaviour: Book book1 = make_book(); Book book2 = make_book(); book1.title() == book2.title() && book1.author() == book2.author() is true, but book1 == book2 is false, because it so happens that they refer to two distinct remote objects. This is a precedent in STD and boost (unwanted, if you asked my opinion). I do not know what Widget in X Window System does, but I know that not every library in C++ is worth following, and its design worth promoting in Boost. 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.

This is a safe subset of shared_ptr usages.

...

[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.

This works in most of the cases, but consider: class AudioBook // no inheritance { // some stuff operator Book() conts; // convert to your Book // no opereator== because we want to use yours }; AudioBook a; Book b; b == a; // works a == b; // ERROR

...

[4] This page:

...
http://yet-another-user.github.io/boost.pimpl/the_ boost_pimpl_library/pimpl__null___and_strongly_exception_ safe_constructors.html 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.

if you are using it like this: shared_ptr<const DB> make_DB(); Then it is fine I guess, although I would rather create a non-copyable value object in one place and pass around references to it (may not be always possible, I guess). But it is superior to pimpl::valu_semantics in two ways: 1. It does not allow mutability 2. It has value semantics (a pointer --even smart-- is a value: it represents an index in a big array -- free store memory). I hope I managed to communicate my thoughts more clearly. Regards, &rzej

Rob Stewart

2:08 p.m.

On June 7, 2014 8:05:39 AM EDT, Andrzej Krzemienski wrote:

...

2014-06-06 3:51 GMT+02:00 Vladimir Batov :

...
On 06/06/2014 07:21 AM, Andrzej Krzemienski wrote:

I still claim that pimpl::pointer_semantics breaches value semantics expectations in an unprecedented way. Both non-const refs and shared_ptr, and raw pointer and normal value semantic classes have one property in common: Their operator== reflects their *salient attributes*. Sorry for a fancy term. I borrowed it from John Lakos: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2479.pdf, https://github.com/boostcon/cppnow_presentations_2014/blob/master/files/accu...

The point of the Pimpl Idiom is to move what would be private data members out of the header. Any semantic thing you would have done, had the members been declared in the header, should be possible after applying the Pimpl Idiom. If value_semantics precludes proper equality comparisons, that's a major problem. If it doesn't preclude them, but changes the default behavior, relative to non-Pimpl classes, it's unfortunate. ___ Rob (Sent from my portable computation engine)

Andrzej Krzemienski

2:19 p.m.

2014-06-07 16:08 GMT+02:00 Rob Stewart :

...

On June 7, 2014 8:05:39 AM EDT, Andrzej Krzemienski wrote:

...
2014-06-06 3:51 GMT+02:00 Vladimir Batov :

...
On 06/06/2014 07:21 AM, Andrzej Krzemienski wrote:

I still claim that pimpl::pointer_semantics breaches value semantics expectations in an unprecedented way. Both non-const refs and shared_ptr, and raw pointer and normal value semantic classes have one property in common: Their operator== reflects their *salient attributes*. Sorry for a fancy term. I borrowed it from John Lakos: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2479.pdf,

https://github.com/boostcon/cppnow_presentations_2014/blob/master/files/accu...

The point of the Pimpl Idiom is to move what would be private data members out of the header. Any semantic thing you would have done, had the members been declared in the header, should be possible after applying the Pimpl Idiom. If value_semantics precludes proper equality comparisons, that's a major problem. If it doesn't preclude them, but changes the default behavior, relative to non-Pimpl classes, it's unfortunate.

...

From what I understand the, pimpl::value_semantics part behaves intuitively. It is just the other part of the library:

pimpl::pointer_semantics that has the controversial behaviour, and I wouldn't even call it "pointer semantics", because pointers expose value semantics. Regards, &rzej

Bjorn Reese

6 Jun 6 Jun

9:17 a.m.

On 06/05/2014 11:21 PM, 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

My main use case for pimpl is also to avoid header inclusion, but for a different reason. When wrapping third-party software, you usually want to avoid exposing their symbols in your own headers. For instance, you may have a C header full of macros that could clash with symbols elsewhere. By limiting the inclusion of those third-party headers to the .cpp file with the pimpl, you do not pollute the global "namespace".

Mostafa

10:12 a.m.

On Thu, 05 Jun 2014 14:21:04 -0700, Andrzej Krzemienski wrote:

...

It looks from the docs that 'pointer_semantics' implements something like a flyweight. This is so against value semantics.

Not to sound snarky, but pointer semantics is exactly that, pointer semantics, I don't expect it to behave like 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.

Unless I'm dealing with pimpl'd classes, then I don't hold that expectation. Granted, it may not be clear from the type name that the type in question is pimpl'd, but I chalk that up to poor naming and language flexibility. The same issue can easily arise with the following bare bone C++ code: // In some header file. typedef some_type * widget; // In some other header file. widget foo(); When dealing with pimpl'd classes, the expected norm (in my experience) for making deep copies is to use the clone function.

Sohail Somani

12:03 p.m.

On 06/06/2014 6:12 AM, Mostafa wrote:

...

When dealing with pimpl'd classes, the expected norm (in my experience) for making deep copies is to use the clone function.

Why?

Mostafa

12:41 p.m.

On Fri, 06 Jun 2014 05:03:21 -0700, Sohail Somani wrote:

...

On 06/06/2014 6:12 AM, Mostafa wrote:

...
When dealing with pimpl'd classes, the expected norm (in my experience) for making deep copies is to use the clone function.

Why?

Existing convention, Java (clone), C# (Clone), Objective-C (copy), and the RogueWave suite of C++ libraries (clone). Maybe this is just me, but I view pimpl'd classes much like C#/Java references.

Sohail Somani

12:57 p.m.

On 06/06/2014 8:41 AM, Mostafa wrote:

...

On Fri, 06 Jun 2014 05:03:21 -0700, Sohail Somani wrote:

...
On 06/06/2014 6:12 AM, Mostafa wrote:

...
When dealing with pimpl'd classes, the expected norm (in my experience) for making deep copies is to use the clone function.

Why?

Existing convention, Java (clone), C# (Clone), Objective-C (copy), and the RogueWave suite of C++ libraries (clone). Maybe this is just me, but I view pimpl'd classes much like C#/Java references.

C++ has value semantics as a core feature so if your project has a different convention, that will surprise a C++ programmer who started programming in the last 5-10 years. RogueWave and friends were initially written years ago when we didn't know what we know now about writing good C++ software. I used it and didn't enjoy it, but I'm sure it has improved since then. That being said, projects have conventions and if it is a convention for you, that's fine. I wouldn't make everyone use this convention through Boost.Pimpl though. Sohail

Andrey Semashev

1:01 p.m.

On Friday 06 June 2014 08:57:57 Sohail Somani wrote:

...

On 06/06/2014 8:41 AM, Mostafa wrote:

...
On Fri, 06 Jun 2014 05:03:21 -0700, Sohail Somani

wrote:

...
On 06/06/2014 6:12 AM, Mostafa wrote:

...
When dealing with pimpl'd classes, the expected norm (in my experience) for making deep copies is to use the clone function.

Why?

Existing convention, Java (clone), C# (Clone), Objective-C (copy), and the RogueWave suite of C++ libraries (clone). Maybe this is just me, but I view pimpl'd classes much like C#/Java references.

C++ has value semantics as a core feature so if your project has a different convention, that will surprise a C++ programmer who started programming in the last 5-10 years.

RogueWave and friends were initially written years ago when we didn't know what we know now about writing good C++ software. I used it and didn't enjoy it, but I'm sure it has improved since then.

That being said, projects have conventions and if it is a convention for you, that's fine. I wouldn't make everyone use this convention through Boost.Pimpl though.

I think both approaches have their cons and pros, and a good pimpl library should support both. PS: I didn't look at the proposed library.

Mostafa

2:06 p.m.

On Fri, 06 Jun 2014 05:57:57 -0700, Sohail Somani wrote:

...

That being said, projects have conventions and if it is a convention for you, that's fine. I wouldn't make everyone use this convention through Boost.Pimpl though.

I agree with you, and that's why if you go back and look at the context of my response, I was writing with respect to pimpl<...>::pointer_semantics. No where did I make any suggestions on the use of pimpl<...>::value_semantics.

Sohail Somani

2:16 p.m.

On 06/06/2014 10:06 AM, Mostafa wrote:

...

On Fri, 06 Jun 2014 05:57:57 -0700, Sohail Somani wrote:

...
That being said, projects have conventions and if it is a convention for you, that's fine. I wouldn't make everyone use this convention through Boost.Pimpl though.

I agree with you, and that's why if you go back and look at the context of my response, I was writing with respect to pimpl<...>::pointer_semantics. No where did I make any suggestions on the use of pimpl<...>::value_semantics.

Glad we agree! I may have accidentally snipped out some context when I quoted you originally: "When dealing with pimpl'd classes, the expected norm (in my experience) for making deep copies is to use the clone function."

3915

Age (days ago)

3917

Last active (days ago)

List overview

Download

15 comments

8 participants

participants (8)

Andrey Semashev
Andrzej Krzemienski
Bjorn Reese
Gavin Lambert
Mostafa
Rob Stewart
Sohail Somani
Vladimir Batov