Smart pointer implementation

Discussion in 'C++' started by Christopher Benson-Manica, Nov 16, 2004.

  1. 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, Nov 16, 2004
    #1
    1. Advertising

  2. 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, Nov 16, 2004
    #2
    1. Advertising

  3. 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, Nov 16, 2004
    #3
  4. 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, Nov 16, 2004
    #4
  5. "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
    John Harrison, Nov 16, 2004
    #5
  6. 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, Nov 16, 2004
    #6
  7. "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, Nov 16, 2004
    #7
  8. 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, Nov 16, 2004
    #8
  9. 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.
    Christopher Benson-Manica, Nov 16, 2004
    #9
  10. 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.
    Christopher Benson-Manica, Nov 16, 2004
    #10
  11. Christopher Benson-Manica

    PKH Guest

    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" <> 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.
    >
    > --
    > 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, Nov 16, 2004
    #11
  12. "Christopher Benson-Manica" <> wrote in message
    news:cnda9k$stj$...
    > 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 ;(
    >


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

    john
    John Harrison, Nov 16, 2004
    #12
  13. John Harrison <> 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.
    Christopher Benson-Manica, Nov 16, 2004
    #13
  14. "Christopher Benson-Manica" <> wrote in message
    news:cndi4e$jj$...
    > John Harrison <> 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
    John Harrison, Nov 16, 2004
    #14
  15. John Harrison <> 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.
    Christopher Benson-Manica, Nov 16, 2004
    #15
  16. "Christopher Benson-Manica" <> wrote in message
    news:cndkbq$nj$...
    > John Harrison <> 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
    John Harrison, Nov 16, 2004
    #16
  17. Christopher Benson-Manica

    Gactimus Guest

    Encryption using an offset file

    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::eek: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::eek: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;
    }
    Gactimus, Nov 17, 2004
    #17
  18. Christopher Benson-Manica wrote:
    >
    > John Harrison <> 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
    Karl Heinz Buchegger, Nov 17, 2004
    #18
  19. Karl Heinz Buchegger wrote:
    >
    > Christopher Benson-Manica wrote:
    > >
    > > John Harrison <> 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
    Karl Heinz Buchegger, Nov 17, 2004
    #19
  20. Christopher Benson-Manica <> wrote in message news:<cndi4e$jj$>...
    > John Harrison <> 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
    Michiel Salters, Nov 17, 2004
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. MotoK
    Replies:
    59
    Views:
    1,785
    Keith Thompson
    Sep 15, 2006
  2. maadhuu

    Smart pointer implementation.

    maadhuu, Dec 3, 2005, in forum: C++
    Replies:
    3
    Views:
    468
    Jonathan Mcdougall
    Dec 3, 2005
  3. coala
    Replies:
    3
    Views:
    360
    coala
    Sep 6, 2006
  4. coala
    Replies:
    1
    Views:
    576
    Victor Bazarov
    Sep 6, 2006
  5. Hicham Mouline
    Replies:
    100
    Views:
    2,021
    Noah Roberts
    Aug 25, 2009
Loading...

Share This Page