«

Is there anything wrong with my attempt (below) at implementing
something resembling a smart pointer?

template < class T >
class SmartPointer
{
private:
T *t;

public:
SmartPointer() {t=new T();}
SmartPointer( const SmartPointer<T> &rhs ) {t=rhs.t;}
T& operator*() const {return(*t);}
T* operator->() const {return(t);}
SmartPointer<T>& operator= ( const SmartPointer<T> &rhs ) {t=rhs.t; return(*this);}
~SmartPointer() {delete t;}
};

The program I wrote to test this crashes, and I wouldn't be at all
surprised to learn that I've made a mistake here somewhere. And no, I
can't use Boost for this.

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.

Christopher Benson-Manica wrote:
>
> Is there anything wrong with my attempt (below) at implementing
> something resembling a smart pointer?
>
> template < class T >
> class SmartPointer
> {
> private:
> T *t;
>
> public:
> SmartPointer() {t=new T();}
> SmartPointer( const SmartPointer<T> &rhs ) {t=rhs.t;}
> T& operator*() const {return(*t);}
> T* operator->() const {return(t);}
> SmartPointer<T>& operator= ( const SmartPointer<T> &rhs ) {t=rhs.t; return(*this);}
> ~SmartPointer() {delete t;}
> };
>
> The program I wrote to test this crashes, and I wouldn't be at all
> surprised to learn that I've made a mistake here somewhere. And no, I
> can't use Boost for this.

Yep. You copy constructor is wrong. It does a memberwise copy, when
it should do a deep copy.

SmartPointer( const SmartPointer<T> &rhs ) {t = new T( rhs.t ); }

same for your assignment operator

--
Karl Heinz Buchegger

Karl Heinz Buchegger wrote:
>
> Christopher Benson-Manica wrote:
> >
> > Is there anything wrong with my attempt (below) at implementing
> > something resembling a smart pointer?
> >
> > template < class T >
> > class SmartPointer
> > {
> > private:
> > T *t;
> >
> > public:
> > SmartPointer() {t=new T();}
> > SmartPointer( const SmartPointer<T> &rhs ) {t=rhs.t;}
> > T& operator*() const {return(*t);}
> > T* operator->() const {return(t);}
> > SmartPointer<T>& operator= ( const SmartPointer<T> &rhs ) {t=rhs.t; return(*this);}
> > ~SmartPointer() {delete t;}
> > };
> >
> > The program I wrote to test this crashes, and I wouldn't be at all
> > surprised to learn that I've made a mistake here somewhere. And no, I
> > can't use Boost for this.
>
> Yep. You copy constructor is wrong. It does a memberwise copy, when
> it should do a deep copy.
>
> SmartPointer( const SmartPointer<T> &rhs ) {t = new T( rhs.t ); }
>
> same for your assignment operator

Actually the situation for the assignment operator is worse.
As it is now, it additionally leaks memory

--
Karl Heinz Buchegger

Karl Heinz Buchegger <> spoke thus:

> Yep. You copy constructor is wrong. It does a memberwise copy, when
> it should do a deep copy.

Right, right... d'oh...

> SmartPointer( const SmartPointer<T> &rhs ) {t = new T( rhs.t ); }

With delete t; coming before the assignment, of course Thanks!

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.

"Christopher Benson-Manica" <> wrote in message
news:cnd76m$sni$...
> Is there anything wrong with my attempt (below) at implementing
> something resembling a smart pointer?
>
> template < class T >
> class SmartPointer
> {
> private:
> T *t;
>
> public:
> SmartPointer() {t=new T();}
> SmartPointer( const SmartPointer<T> &rhs ) {t=rhs.t;}
> T& operator*() const {return(*t);}
> T* operator->() const {return(t);}
> SmartPointer<T>& operator= ( const SmartPointer<T> &rhs ) {t=rhs.t;
> return(*this);}
> ~SmartPointer() {delete t;}
> };
>
> The program I wrote to test this crashes, and I wouldn't be at all
> surprised to learn that I've made a mistake here somewhere. And no, I
> can't use Boost for this.
>

Your smart pointer is not very smart (sorry). It is basically no different
from a raw pointer. When you copy, you end up with two pointers to the same
object and no idea when it is safe to delete the pointer.

The commonest implementation of a smart pointer uses reference counting to
overcome this problem. The reference count counts how many copies of the
smart pointer you have. When the count reaches zero you know it is safe to
delete the pointer. Reference counting comes in two varieties, intrusive and
non-intrusive. With intrusive reference counting the count is held in the
pointed-to object itself, with non-intrusive the reference count is help
outside the pointed-to object.

Have a look on the web for reference counted smart pointer implementations.
Then have a look at boost, when they have high quality implementations of
both types (but a bit more complex than your typical implementation).

John

Christopher Benson-Manica wrote:
>
> Karl Heinz Buchegger <> spoke thus:
>
> > Yep. You copy constructor is wrong. It does a memberwise copy, when
> > it should do a deep copy.
>
> Right, right... d'oh...
>
> > SmartPointer( const SmartPointer<T> &rhs ) {t = new T( rhs.t ); }
>
> With delete t; coming before the assignment, of course Thanks!
>

Definitly not.
This is a constructor. t has no meaningful value.
Actually the whole thing should be written as

SmartPointer( const SmartPointer<T> &rhs ) : t( new T( rhs.t ) ) {}

--
Karl Heinz Buchegger

"Karl Heinz Buchegger" <> wrote in message
news:...
> Christopher Benson-Manica wrote:
>>
>> Is there anything wrong with my attempt (below) at implementing
>> something resembling a smart pointer?
>>
>> template < class T >
>> class SmartPointer
>> {
>> private:
>> T *t;
>>
>> public:
>> SmartPointer() {t=new T();}
>> SmartPointer( const SmartPointer<T> &rhs ) {t=rhs.t;}
>> T& operator*() const {return(*t);}
>> T* operator->() const {return(t);}
>> SmartPointer<T>& operator= ( const SmartPointer<T> &rhs ) {t=rhs.t;
>> return(*this);}
>> ~SmartPointer() {delete t;}
>> };
>>
>> The program I wrote to test this crashes, and I wouldn't be at all
>> surprised to learn that I've made a mistake here somewhere. And no, I
>> can't use Boost for this.
>
> Yep. You copy constructor is wrong. It does a memberwise copy, when
> it should do a deep copy.
>
> SmartPointer( const SmartPointer<T> &rhs ) {t = new T( rhs.t ); }
>
> same for your assignment operator
>

A smart pointer implemented like that doesn't have many uses. Most of the
time I would prefer to simply create and copy objects than use such a smart
pointer.

john

John Harrison wrote:
>
[snip]
>
> A smart pointer implemented like that doesn't have many uses. Most of the
> time I would prefer to simply create and copy objects than use such a smart
> pointer.

Agreed.
But if it makes the OP happy


--
Karl Heinz Buchegger

Karl Heinz Buchegger <> spoke thus:

> This is a constructor. t has no meaningful value.

I realized that after I posted...

> Actually the whole thing should be written as

> SmartPointer( const SmartPointer<T> &rhs ) : t( new T( rhs.t ) ) {}

That'd be nice, except that T doesn't have a copy constructor ;(

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.

Karl Heinz Buchegger <> spoke thus:

> But if it makes the OP happy

Well, the whole exercise is necessary because I want to put some
objects in standard containers, but they don't fit in the containers
directly (thanks to their use of stupid Borland extensions).
Therefore I figured this would be the easiest way to put them in a
standard container...

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.