
On May 30, 2016 1:42:39 AM EDT, Vladimir Batov
On 2016-05-30 14:18, Gavin Lambert wrote:
On 30/05/2016 15:16, Vladimir Batov wrote:
So, in your view, should we try and have pimpl in Boost? I think we should as it'd be useful IMO...
As I said at the start, yes, I think this has potential usefulness and it would be nice to get something like it into Boost.
Excellent.
I, too, think that something that simplifies The Pimpl Idiom could be a good addition to Boost.
As it's the template specialisation which causes most of my concern, my main query is whether it's really necessary or if it could be structured differently to avoid this instead.
... Taking the Book example from the docs... ... we get something like this:
namespace library { namespace detail { ... } }
namespace boost { template<> struct pimpllibrary::Book::implementation { implementation(string const& the_title, string const& the_author) : title(the_title), author(the_author), price(0) {} ... bool check_isbn_10_digit() { ... } ... string title; string author; int price; }; }
namespace library { Book::Book(string const& title, string const& author) : pimpl_type(title, author) {}
string const& Book::author() const { return (*this)->author; } ... } ...
Yes, I understand. That's why I said before and still insist now that that's style...
Adding The Pimpl Idiom to a class means splitting the implements across two class: one public and one private. Even then, the private class is, normally nested in the public class so they are tightly related. With your library, that split is carried further by splitting the code across two distinct namespaces. That is, indeed, a style issue, since the code still does the same work. Nevertheless it seems wrong to write the implementation details of one's own class in the (proposed) boost namespace. I've not given any thought to alternatives, so I can only criticize your solution at present. ___ Rob (Sent from my portable computation engine)