Subject: Re: [boost] [pimpl] Mini Review
From: Ivan Le Lann (ivan.lelann_at_[hidden])
Date: 2011-05-26 06:15:55
----- "Artyom Beilis" <artyomtnk_at_[hidden]> a écrit :
> Hello All,
>
> Because a pre-review runs this days in collaborator tool I still want
> to provide some feedback on this library I was looking on long
> time ago.
>
> - What is your evaluation of the design?
>
> ---------------------------------------------
> This is the major problem of the library:
> It tries to do simple things overcomplicated.
> ---------------------------------------------
>
>
> Take as an example:
>
> struct Book : public pimpl<Book>::value_semantics { ... };
>
> Is an attempt to do things in "fancy" way where it is not needed.
>
> struct Book {
> public:
> private:
> struct implementation;
> smart_ptr<implementation> pimpl_;
> };
>
> Is much clearer.
Your code would rather simulate pimpl<>::pointer_semantics.
And you do not mention the boilerplate code generated.
>
> The proposed solution is overcomplicated hard to read and in general
> makes
> more troubles then helps, makes the code less readable by average
> programmers.
>
> ------------------------------------
> shared_ptr as pointer model is Wrong
> ------------------------------------
>
(snip lots of smart_ptr related lines)
Unless you want pointer_semancics ...
As for value_semantics, shared_ptr is not used.
> 2. clone_ptr - pointer that clones underlying object on its copy.
> i.e. something like
>
> copy_ptr(copy_ptr const &other) : p_(other->clone()) {}
Precisely what seems to be used in the library for value_semantics.
>
> The only advantage I can see in boost.pimpl over scoped_ptr or
> auto_ptr
> is that it does not require explicit destructor.
An important advantage as many developpers get trapped here.
And it is "only" a warning to do so ...
> Spare Pointer use case
> ------------------------
>
> Another important use case in the case when pimpl is used to handle
> ABI
> stability and
> in general the object has spare "pimpl" or d-pointer for future uses.
>
> Consider:
>
> struct book {
> public:
> book();
> book(book const &);
> ~book(book const &);
> book const &operator=(book const &);
> std::string title() const;
> std::string author() const;
> void title(std::string const &);
> void author(std::string const &);
> private:
> std::string author_;
> std::string title_;
> struct data;
> copy_ptr<data> d;
> };
>
>
> Now in book.cpp we define data as
> struct book::data {};
And with Pimpl, you would put author_ and title_ in book::data.
I failed to see the point here. You seem to say that Pimpl cannot
be used for ABI stability, which seems wrong to me.
Regards,
Ivan.