Subject: Re: [boost] Review Request: impl_ptr (pimpl)
From: Vladimir Batov (Vladimir.Batov_at_[hidden])
Date: 2017-08-19 22:46:27
On 2017-08-19 23:55, Mario Lang wrote:
> Vladimir Batov via Boost <boost_at_[hidden]> writes:
> I got interested enough by this posting so that I ended up doing my own
> mini-review just right now :-). And I just wanted to let you know (and
> encourage you) that I like this one alot. Keeping the public interface
> free of boilerplate is quite nice!
Thank you for your input. Much appreciated... although somewhat early.
:-) It was pointed out to me that Boost review procedures changed and a
proposal has to be endorsed before it can move to the review stage. So
far I do not see it happening... maybe because the list is quiet
(overall), maybe because iml_ptr is not good enough to attract that
support/endorsement, maybe giving "endorsement" puts too much pressure
and people shy away from it. We'll see.
> I think for a full boost review, you will probably need to work on:
> * The documentation
> While I gave it a read, I ended up skipping to your test case because
> I wanted to see working example code.
> While the current docs have quite some examples, they spend a lot
> screen space with showing what impl_ptr tries to replace, without
> showing a good motivating positive example of how it actually looks
> when employed. It also feels like it is spending some time with
> marketing speak, while not really showing what it is trying to sell:
Yes, I hear you... and I (at least partially) agree with you. First, no
code or documentation will ever be to everyone's liking... unless
everyone writes their own. Secondly, the documentation reflects many
discussions we've had on this list about pimpl. For ex., I had to
explicitly add what you call "marketing speak" to plainly explain why
IMO pimpl is important. I had to address numerous "why bother"
dismissing comments. What you call "marketing speak" I call an
architectural view of a project. IMO it's quite different from
developer's view that you seem to favor. All views are important... and
all of them IMO need to be mentioned to cater for and to convince
different readers. Surely, you (as a developer) is free to skip the
"marketing speak". Somebody else (an architect and code-reviewer) might
decide to skip other bits. No drama. Does it make sense?
> The separation between the public interface in a hpp and the template
> specialization in a cpp. I think the motivating example should
> clearly show what part of the code goes where.
Isn't what I did in third and fourth chapters? That said, I'll never be
able to write the way you (or somebody else) like it. So, if you feel
really strongly about it, please feel free to re-write it as you feel
proper and submit. I am more than happy to consider and take others
help. Unfortunately, that'll reverse our positions as I (and potential
reviewers) might be saying "nuh, not enough "marketing speak" and such.
No pressure... but that's an option to shape it up to your liking.
> * Namespace:
> While the alias is good, I think the whole code should be moved to
> boost::, and the detail to boost::detail::impl_ptr::.
> I am sort of guessing this will come up in the review.
> I might be interested to help with that and submit the renaming PR if
> you are open to that.
1) I do agree that the "whole code should be moved to boost::"... when
it is part of Boost. Currently it is not. Currently I am on the earliest
stage of gathering interest and waiting for "endorsement" and the review
2) As I think I mentioned before impl_ptr is not part of any namespace
by design... because the way proxy and implementation are connected, the
specialization of impl_ptr<>::implementation. As I also mentioned it is
IMO only a "problem" for "purists" but not for "practitioners" as from
outside it is all kosher and accessed via boost::impl_ptr.
You might be interested in searching for "Pimpl Again?" thread on this
list for prev. discussions.
Still, it you know a way around it and willing to submit your
suggestions, I am certainly open for and most welcome it. But just to
state the obvious -- as you do not like every line of code/doc that I
write -- that goes both ways.