smart pointers

[keith]

When I am using the private implementation idiom, is there anything to
be gained by using a boost::scoped_ptr as opposed to a auto_ptr? I
confess I'm not that clear on the differences...

 

[Cholo Lennon]

When I am using the private implementation idiom, is there anything to
be gained by using a boost::scoped_ptr as opposed to a auto_ptr? I
confess I'm not that clear on the differences...

If your classes are noncopyable: a const auto_ptr to hold the pimpl is
almost the same than scoped_ptr: scoped_ptr can be 'reset' to change
the pointee pimpl. You can't change the pointee pimpl with const
auto_ptr

If your classes are copyable: auto_ptr can't be const and non const
auto_ptr leads you to the disaster due to auto_ptr's transfer
ownership. When my classes are copyable I prefer using shared_ptr to
hold the pimpl (to avoid the manual coding of pimpl copy)

Regards

 

[keith]

If your classes are noncopyable: a const auto_ptr to hold the pimpl is
almost the same than scoped_ptr: scoped_ptr can be 'reset' to change
the pointee pimpl. You can't change the pointee pimpl with const
auto_ptr

If your classes are copyable: auto_ptr can't be const and non const
auto_ptr leads you to the disaster due to auto_ptr's transfer
ownership. When my classes are copyable I prefer using shared_ptr to
hold the pimpl (to avoid the manual coding of pimpl copy)

I'm unclear exactly what non-copyable means in this context. The
following seems to copy just fine.

Here's part of a wrapper class for an MD5 hash I've been playing with
to get to grips with this pimpl stuff:

//-----------------------------------
#include <boost/scoped_ptr.hpp>
#include <stdint.h>
#include "hash.h"

class MD5Private;
class MD5 : public virtual Hash
{
public:
using Hash::hash;
using Hash::end;
using Hash::digest;

MD5();
MD5(const MD5& h);
MD5& operator= (const MD5& h) throw();
virtual ~MD5() throw();

virtual void begin () throw();
virtual void hash (const uint8_t* data, const uint_32t& len)
throw();
virtual void end (const uint8_t* hval) throw();

private:
boost::scoped_ptr<MD5Private> p_;
};
//--------------------------

and in the relevant bits of the class definition I currently have:

//--------------------------
class MD5Private {
public:
uint32_t i[2];
uint32_t buf[4];
uint8_t in[64];
uint8_t digest[16];
};

// New object, so initialise
MD5::MD5() : p_(new MD5Private()) { begin(); }

// Copy object, so do _not_ initialise
MD5::MD5(const MD5& other) :
Hash(other),
p_(other.p_.get() ? new MD5Private(*other.p_) : NULL)
{ }

MD5::~MD5() throw() { }

MD5&
MD5:perator= (const MD5& rhs) throw()
{
MD5 temp(rhs);
swap(p_, temp.p_);
return *this;
}
//<etc, etc>
//--------------------------

Are you saying that by using a boost::scoped_ptr this should not work?

 

[Cholo Lennon]

On Feb 14, 10:27 am, (e-mail address removed) wrote:

If your classes are noncopyable: a const auto_ptr to hold the pimpl is
almost the same than scoped_ptr: scoped_ptr can be 'reset' to change
the pointee pimpl. You can't change the pointee pimpl with const
auto_ptr

If your classes are copyable: auto_ptr can't be const and non const
auto_ptr leads you to the disaster due to auto_ptr's transfer
ownership. When my classes are copyable I prefer using shared_ptr to
hold the pimpl (to avoid the manual coding of pimpl copy)

I'm unclear exactly what non-copyable means in this context. The
following seems to copy just fine.

Here's part of a wrapper class for an MD5 hash I've been playing with
to get to grips with this pimpl stuff:

//-----------------------------------
#include <boost/scoped_ptr.hpp>
#include <stdint.h>
#include "hash.h"

class MD5Private;
class MD5 : public virtual Hash
{
public:
using Hash::hash;
using Hash::end;
using Hash::digest;

MD5();
MD5(const MD5& h);
MD5& operator= (const MD5& h) throw();
virtual ~MD5() throw();

virtual void begin () throw();
virtual void hash (const uint8_t* data, const uint_32t& len)
throw();
virtual void end (const uint8_t* hval) throw();

private:
boost::scoped_ptr<MD5Private> p_;};

//--------------------------

and in the relevant bits of the class definition I currently have:

//--------------------------
class MD5Private {
public:
uint32_t i[2];
uint32_t buf[4];
uint8_t in[64];
uint8_t digest[16];

};

// New object, so initialise
MD5::MD5() : p_(new MD5Private()) { begin(); }

// Copy object, so do _not_ initialise
MD5::MD5(const MD5& other) :
Hash(other),
p_(other.p_.get() ? new MD5Private(*other.p_) : NULL)
{ }

MD5::~MD5() throw() { }

MD5&
MD5:perator= (const MD5& rhs) throw()
{
MD5 temp(rhs);
swap(p_, temp.p_);
return *this;}

//<etc, etc>
//--------------------------

Are you saying that by using a boost::scoped_ptr this should not work?

No, I'm saying that using scoped_ptr you have to define copy ctor and
operator= like you did. Your code seems to be perfectly valid.

and... I'm sorry, I did a mistake when I said:

"When my classes are copyable I prefer using shared_ptr to
hold the pimpl (to avoid the manual coding of pimpl copy)"

It should have been:

"When my classes are copyable and the internals details have to be
shared I use shared_ptr (with shared_ptr you don't have to define copy
ctor and operator="

(It's very common for me to use classes with internal shared details,
this was the source of my mistake).


BTW, I prefer using a nested private class for pimpl data:

class MD5 {

private:
class Private;
boost::scoped_ptr<Private> p_;
};

....

class MD5:rivate { ... };



Regards

 

[keith]

No, I'm saying that using scoped_ptr you have to define copy ctor and
operator= like you did. Your code seems to be perfectly valid.

Thanks. I think I've got it now.

BTW, I prefer using a nested private class for pimpl data:

class MD5 {

private:
class Private;
boost::scoped_ptr<Private> p_;

};

...

class MD5:rivate { ... };

That's neater - I like it. Thanks again.