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.

 

[Karl Heinz Buchegger]

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]

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

 

[Christopher Benson-Manica]

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!

 

[John Harrison]

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]

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


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 ) ) {}

 

[John Harrison]

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]

[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

 

[Christopher Benson-Manica]

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]

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

 

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

 

[John Harrison]

I realized that after I posted...



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]

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

 

[John Harrison]

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]

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

 

[John Harrison]

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

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]

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.

 

[Michiel Salters]

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