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

[Karl Heinz Buchegger]

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

[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
kb******@gascad.at

[Christopher Benson-Manica]

Karl Heinz Buchegger <kb******@gascad.at> 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.

[John Harrison]

"Christopher Benson-Manica" <at***@nospam.cyberspace.org> wrote in message
news:cn**********@chessie.cirr.com...

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

[Karl Heinz Buchegger]

Christopher Benson-Manica wrote:

Karl Heinz Buchegger <kb******@gascad.at> 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
kb******@gascad.at

[John Harrison]

"Karl Heinz Buchegger" <kb******@gascad.at> wrote in message
news:41**************@gascad.at...

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

[Karl Heinz Buchegger]

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

[Christopher Benson-Manica]

Karl Heinz Buchegger <kb******@gascad.at> 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.

[Christopher Benson-Manica]

Karl Heinz Buchegger <kb******@gascad.at> 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.

[PKH]

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.

"Christopher Benson-Manica" <at***@nospam.cyberspace.org> wrote in message
news:cn**********@chessie.cirr.com...

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.

[John Harrison]

"Christopher Benson-Manica" <at***@nospam.cyberspace.org> wrote in message
news:cn**********@chessie.cirr.com...

Karl Heinz Buchegger <kb******@gascad.at> 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 ;(

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

john

[Christopher Benson-Manica]

John Harrison <jo*************@hotmail.com> spoke thus:

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

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

[John Harrison]

"Christopher Benson-Manica" <at***@nospam.cyberspace.org> wrote in message
news:cn*********@chessie.cirr.com...

John Harrison <jo*************@hotmail.com> spoke thus:

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

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

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

john

[Christopher Benson-Manica]

John Harrison <jo*************@hotmail.com> spoke thus:

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

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

[John Harrison]

"Christopher Benson-Manica" <at***@nospam.cyberspace.org> wrote in message
news:cn*********@chessie.cirr.com...

John Harrison <jo*************@hotmail.com> spoke thus:

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

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

If you intend to put those object into a vector, then you are going to find
it hard to avoid them being referred to more than once. For instance suppose
you vector contains four objects

v[0] is a
v[1] is b
v[2] is c
v[3] is d

Then suppose you erase b, the vector is likely to copy v[2] onto v[1], then
v[3] onto v[2] then destroy v[3]. So counting all the intermediate stages
you will get a vector that looks like this

v[0] is a
v[1] is c
v[2] is c
v[3] is d

then

v[0] is a
v[1] is c
v[2] is d
v[3] is d

then finally

v[0] is a
v[1] is c
v[2] is d

So your class must be able to deal with more than one reference at a time.
Reference counting would make copy operations like the above much more
efficient as well.

john

[Gactimus]

Here is a program that encodes and decodes a text file. What I need to do
is write a C++ program that requests 3 different file names. One filename
is for the source file to be encoded, another is the name of the 'encoded'
output file, and the final filename is an offset file. On a character by
character basis, the program needs to look at the first character of the
source file and offset it by the first character in the offset file. The
second character in the source is offset by the second character in the
secret offset file. I need to deal with the possibility that your source
is longer than the offset file in which case it just needs start over or
repeat the secret offset file from the beginning.

The one thing I am having trouble with is offsetting the source file with
the offset file. Here is my code. I need to implement the offset file. Any
ideas?
#include <iostream>
#include <fstream>
#include <cstdlib>
#include <string>
using namespace std;

#define cypher (char) ASCII + 1
#define decypher (char) ASCII - 1

int main()
{
char ASCII;
cout << "1) Encode\n2) Decode\n<q to quit>: ";
int coding;
cin >> coding;

if(coding == 1)
{
ifstream source;
ofstream clone;

source.open("lazy_dog.txt", ios::in);
clone.open("cypher.txt", ios::out | ios::trunc);

while(!source.eof())
{
char NewASCII;
ASCII = source.get();
if (source.fail())
return 0;
NewASCII = cypher;
clone.put(NewASCII);
}

source.close();
clone.close();
return 0;
}

else if(coding == 2)
{
ifstream origin;
ofstream copy;

origin.open("cypher.txt", ios::in);
copy.open("decypher.txt", ios::out | ios::trunc);

while(!origin.eof())
{
char NewASCII;
ASCII = origin.get();
if (origin.fail())
return 0;
NewASCII = decypher;
copy.put(NewASCII);
}

origin.close();
copy.close();
return 0;
}

else
return 0;
}

[Karl Heinz Buchegger]

Christopher Benson-Manica wrote:

John Harrison <jo*************@hotmail.com> spoke thus:

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

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,

In this case you definitly should disable copying and assignment for
your pointer class.

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

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

private:
SmartPointer( const SmartPointer<T> &rhs ); // not implemented by intent
SmartPointer<T>& operator= ( const SmartPointer<T> &rhs ); // not implemented by intent
};

In this way it is impossible to create a copy of a SmartPointer or assign
to it, not even by accident.

--
Karl Heinz Buchegger
kb******@gascad.at

[Karl Heinz Buchegger]

Karl Heinz Buchegger wrote:

Christopher Benson-Manica wrote:

John Harrison <jo*************@hotmail.com> spoke thus:

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

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,

In this case you definitly should disable copying and assignment for
your pointer class.

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

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

private:
SmartPointer( const SmartPointer<T> &rhs ); // not implemented by intent
SmartPointer<T>& operator= ( const SmartPointer<T> &rhs ); // not implemented by intent
};

In this way it is impossible to create a copy of a SmartPointer or assign
to it, not even by accident.

I just read in another posting that you want to put those pointers into a
standard container. That will not work with the above.
I think your best bet would be to get an implementation of a reference
counted smart pointer for that task.
--
Karl Heinz Buchegger
kb******@gascad.at

[Michiel Salters]

Christopher Benson-Manica <at***@nospam.cyberspace.org> wrote in message news:<cn*********@chessie.cirr.com>...

John Harrison <jo*************@hotmail.com> spoke thus:

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

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

Sounds like boost::shared_ptr<T>

Regards,
Michiel Salters

[Christopher Benson-Manica]

John Harrison <jo*************@hotmail.com> spoke thus:

If you intend to put those object into a vector, then you are going to find
it hard to avoid them being referred to more than once.
Well, unless of course I do it the stupid way I'm doing now ;) See,
with my way, I get
v[0] is a
v[1] is c
v[2] is c
v[3] is d

v[0] is a
v[1] is copy of c
v[2] is c
v[3] is d

(and so on)

It works, but I now see that it's a bad way to do it. Curse Borland
for making me have to do this in the first place ;(

I appreciate your help :)

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

Karl Heinz Buchegger <kb******@gascad.at> spoke thus:

I just read in another posting that you want to put those pointers into a
standard container. That will not work with the above.

Doesn't it...? My current, perhaps bogus, understanding is that my
way works, just very inefficiently:

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

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

So instead of trading references to the same object (which I now see
is the Good Way to do this), the object always gets copied. So
nothing ever gets left behind, but there are also a bunch of
unnecessary memory operations. Well, at least that's how it works in
my fantasy world...

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

Christopher Benson-Manica wrote:

Karl Heinz Buchegger <kb******@gascad.at> spoke thus:

I just read in another posting that you want to put those pointers into a
standard container. That will not work with the above.

Doesn't it...?

The comment of not working has to be seen in the light of
hiding the copy constructor and assignment operator.
The standard container request your objects to be able
to perform those operations.
No copy constructor, no assignment operator -> no use in
a standard container.

--
Karl Heinz Buchegger
kb******@gascad.at

[Christopher Benson-Manica]

Karl Heinz Buchegger <kb******@gascad.at> spoke thus:

No copy constructor, no assignment operator -> no use in
a standard container.

Well, the StupidPointer has both, right? Does it work with simple
parameters (such as int)? Or am I a StupidProgrammer? ;)

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

Christopher Benson-Manica wrote:

Karl Heinz Buchegger <kb******@gascad.at> spoke thus:

No copy constructor, no assignment operator -> no use in
a standard container.

Well, the StupidPointer has both, right? Does it work with simple
parameters (such as int)? Or am I a StupidProgrammer? ;)

It is fine.

The order of events was:
* You said, that you plan to have only one pointer to the object at any time
* I suggested, in order to achieve this, to 'hide' the cctor and the op= by
making them private and not defining them. By doing this you could
*ensure* that only one pointer ever existed to the object.
To be able to ensure something is definitly better then to depend on
the programmers memory to not do it.
* Then it turned out that you want to build collections from this pointer
* In order to do this, the cctor and op= cannot be private, otherwise
you would not be able to use the containers.

--
Karl Heinz Buchegger
kb******@gascad.at