Boost logo

Boost :

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.


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk