Smart pointer implementation

[Christopher Benson-Manica]

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)cybers pace.org | don't, I need to know. Flames welcome.

[Karl Heinz Buchegger]

Re: Smart pointer implementation

Christopher Benson-Manica wrote:[color=blue]
>
> 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.[/color]

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
kbuchegg@gascad .at

[Karl Heinz Buchegger]

Re: Smart pointer implementation

Karl Heinz Buchegger wrote:[color=blue]
>
> Christopher Benson-Manica wrote:[color=green]
> >
> > 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.[/color]
>
> 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[/color]

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

--
Karl Heinz Buchegger
kbuchegg@gascad .at

[Christopher Benson-Manica]

Re: Smart pointer implementation

Karl Heinz Buchegger <kbuchegg@gasca d.at> spoke thus:
[color=blue]
> Yep. You copy constructor is wrong. It does a memberwise copy, when
> it should do a deep copy.[/color]

Right, right... d'oh...
[color=blue]
> SmartPointer( const SmartPointer<T> &rhs ) {t = new T( rhs.t ); }[/color]

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)cybers pace.org | don't, I need to know. Flames welcome.

[John Harrison]

Re: Smart pointer implementation


"Christophe r Benson-Manica" <ataru@nospam.c yberspace.org> wrote in message
news:cnd76m$sni $1@chessie.cirr .com...[color=blue]
> 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.
>[/color]

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

[Karl Heinz Buchegger]

Re: Smart pointer implementation

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

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
kbuchegg@gascad .at

[John Harrison]

Re: Smart pointer implementation


"Karl Heinz Buchegger" <kbuchegg@gasca d.at> wrote in message
news:419A26E1.8 5C6D3D@gascad.a t...[color=blue]
> Christopher Benson-Manica wrote:[color=green]
>>
>> 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.[/color]
>
> 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
>[/color]

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

[Karl Heinz Buchegger]

Re: Smart pointer implementation

John Harrison wrote:[color=blue]
>[/color]
[snip][color=blue]
>
> 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.[/color]

Agreed.
But if it makes the OP happy :-)


--
Karl Heinz Buchegger
kbuchegg@gascad .at

[Christopher Benson-Manica]

Re: Smart pointer implementation

Karl Heinz Buchegger <kbuchegg@gasca d.at> spoke thus:
[color=blue]
> This is a constructor. t has no meaningful value.[/color]

I realized that after I posted...
[color=blue]
> Actually the whole thing should be written as[/color]
[color=blue]
> SmartPointer( const SmartPointer<T> &rhs ) : t( new T( rhs.t ) ) {}[/color]

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)cybers pace.org | don't, I need to know. Flames welcome.

[Christopher Benson-Manica]

Re: Smart pointer implementation

Karl Heinz Buchegger <kbuchegg@gasca d.at> spoke thus:
[color=blue]
> But if it makes the OP happy :-)[/color]

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)cybers pace.org | don't, I need to know. Flames welcome.

[PKH]

Re: Smart pointer implementation

I find smartpointers to be more useful if they are integrated with the
delete-operator so that when the object the smartpointer points to is
deleted, all the smartpointers that point to it are set to point to NULL.
It's a good way to make sure you're not using pointers to deleted memory.

"Christophe r Benson-Manica" <ataru@nospam.c yberspace.org> wrote in message
news:cnd76m$sni $1@chessie.cirr .com...[color=blue]
> 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)cybers pace.org | don't, I need to know. Flames welcome.[/color]

[John Harrison]

Re: Smart pointer implementation


"Christophe r Benson-Manica" <ataru@nospam.c yberspace.org> wrote in message
news:cnda9k$stj $1@chessie.cirr .com...[color=blue]
> Karl Heinz Buchegger <kbuchegg@gasca d.at> spoke thus:
>[color=green]
>> This is a constructor. t has no meaningful value.[/color]
>
> I realized that after I posted...
>[color=green]
>> Actually the whole thing should be written as[/color]
>[color=green]
>> SmartPointer( const SmartPointer<T> &rhs ) : t( new T( rhs.t ) ) {}[/color]
>
> That'd be nice, except that T doesn't have a copy constructor ;(
>[/color]

If T doesn't have a copy ctor, then I think you should definitely be using
reference counting.

john

[Christopher Benson-Manica]

Re: Smart pointer implementation

John Harrison <john_andronicu s@hotmail.com> spoke thus:
[color=blue]
> If T doesn't have a copy ctor, then I think you should definitely be using
> reference counting.[/color]

But the intelligence I want in the pointer is only that it knows how
to copy and delete itself :)

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

[John Harrison]

Re: Smart pointer implementation


"Christophe r Benson-Manica" <ataru@nospam.c yberspace.org> wrote in message
news:cndi4e$jj$ 1@chessie.cirr. com...[color=blue]
> John Harrison <john_andronicu s@hotmail.com> spoke thus:
>[color=green]
>> If T doesn't have a copy ctor, then I think you should definitely be
>> using
>> reference counting.[/color]
>
> But the intelligence I want in the pointer is only that it knows how
> to copy and delete itself :)
>[/color]

Isn't that what reference counting gives you? I might be missing the point.

john

[Christopher Benson-Manica]

Re: Smart pointer implementation

John Harrison <john_andronicu s@hotmail.com> spoke thus:
[color=blue]
> Isn't that what reference counting gives you? I might be missing the point.[/color]

Well, reference counting could, I suppose, give it to me, but I don't
intend for these objects to be referred to more than once, so I'm not
sure if that's what I want. What I have seems to be working, now that
I've fixed the bugs...

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