Re: [boost] [pimpl] Mini Review

Subject: Re: [boost] [pimpl] Mini Review
From: Artyom Beilis (artyomtnk_at_[hidden])
Date: 2011-05-26 08:09:49

> From: Ivan Le Lann <ivan.lelann_at_[hidden]>

> ----- "Artyom Beilis" <artyomtnk_at_[hidden]> a écrit  :
> 
> > 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.
> 

Boost pimpl does not save anything in terms of boilerplate code.

What is the difference between:


    class foo {
    public:
        foo(int x = 0);
        int v() const;
        void v(int x);
    private:
        struct implementation;
        boost::shared_ptr<implementation> pimpl_;
    };

    struct foo::implementation                                               
    {                                                                        
    public:                                                                  
        implementation(int x = 0) : v(x) {}                              
        int v;                                                           
    };                                                                       
                                                                         
                                                                         
    foo::foo(int x) : pimpl_(new implementation(x))                          
    {                                                                        
    }                                                                        
                                                                         
    int foo::v() const                                                       
    {                                                                        
        return pimpl_->v;                                                
    }                                                                        
                                                                         
    void foo::v(int v)                                                       
    {                                                                        
        pimpl_->v=v;                                                     
    }                    


And


    class foo : public pimpl<foo>::pointer_semantics {                       
    public:                                                                  
        foo(int x = 0);
        int v() const ;                                                  
        void v(int x);                                                   
    };    

    template<>
    struct pimpl<foo>::implementation                                        
    {
    public:
        implementation(int x = 0) : v(x) {}                              
        int v;
    };     
           
    foo::foo(int val) : base(val)                                            
    {                                                                        
    }

    int foo::v() const                                                       
    {
        return (**this).v;                                               
    }      
           
    void foo::v(int v)                                                       
    {      
        (**this).v=v;
    }


Not too much, except that first seems to be much more
flexible.

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


No, hold_ptr - for non-copyable objects, clone_ptr for cloneable
objects. Copy ptr is fine.

It does not provide too much semantics.

Also it exposes some details like "operator bool()" to places
that not belong to.

You should not care about whether the internal pointer null or not.

It is an implementation detail!


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

So create hold_ptr/copy_ptr/clone_ptr with information
about how to destroy, copy and clone in the same way
shared_ptr does.

In any case just adding ~foo(); in h and foo::~foo() {} in cpp
solves problem.

Not justified.


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

I explain.

I want to create a class that I may add members in future to it.

So I just put there an opaque pointer that would be used
in future. This is typical use case in many-many places.

I don't want to **derive** my class from some fancy other
class to provide an opaque pointer for future uses.

It is total overkill for something that is not used at all.

So basically I don't want to have title_ and author_ to
be members of data as I don't want to use data right now.

To make it more clean, consider them numeric values like
int with inline accessors that are very fast and efficient
and do not even require memory allocation to copy
an object from one place to other.


Artyom Beilis
--------------
CppCMS - C++ Web Framework:   http://cppcms.sf.net/
CppDB - C++ SQL Connectivity: http://cppcms.sf.net/sql/cppdb/