Reference Counting

Discussion in 'C++' started by Protoman, Jul 10, 2006.

  1. Protoman

    Protoman Guest

    OK, this code compiles, links, and executes, but, how do I setup, like,
    a spinlock to query the DataBase object's status to let the SmrtPtr
    know that the object's been deleted?:

    #pragma once

    class SmrtPtrDB
    {
    public:
    SmrtPtrDB():num(0){}
    ~SmrtPtrDB(){}
    void add(){num++;}
    void sub(){num--;}
    int status(){return num;}
    private:
    int num;
    };

    #pragma once
    #include "SmrtPtrDB.hpp"

    template<class T>
    class SmrtPtr
    {
    public:
    SmrtPtr<T>(T* obj=0):ptr(obj){DataBase.add();)
    SmrtPtr<T>(const SmrtPtr<T>& rhs):ptr(new
    T(*(rhs.ptr))){DataBase.add();}
    ~SmrtPtr<T>(){delete ptr; ptr=0; DataBase.sub();}
    void operator=(T val){*ptr=val;}
    SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs)
    {
    if(this!=&rhs)
    {
    delete ptr;
    ptr(rhs.ptr);
    }
    else return this;
    }
    int status(){DataBase.status();}
    T& operator*()const{return *ptr;}
    T* operator->()const{return ptr;}
    private:
    SmrtPtrDB DataBase;
    T* ptr;
    };

    I'm very confused. Am I implementing this right? Thanks!!!!
     
    Protoman, Jul 10, 2006
    #1
    1. Advertising

  2. Protoman

    Jon Rea Guest

    Protoman wrote:
    > OK, this code compiles, links, and executes, but, how do I setup, like,
    > a spinlock to query the DataBase object's status to let the SmrtPtr
    > know that the object's been deleted?:
    >
    > #pragma once
    >
    > class SmrtPtrDB
    > {
    > public:
    > SmrtPtrDB():num(0){}
    > ~SmrtPtrDB(){}
    > void add(){num++;}
    > void sub(){num--;}
    > int status(){return num;}
    > private:
    > int num;
    > };
    >
    > #pragma once
    > #include "SmrtPtrDB.hpp"
    >
    > template<class T>
    > class SmrtPtr
    > {
    > public:
    > SmrtPtr<T>(T* obj=0):ptr(obj){DataBase.add();)
    > SmrtPtr<T>(const SmrtPtr<T>& rhs):ptr(new
    > T(*(rhs.ptr))){DataBase.add();}
    > ~SmrtPtr<T>(){delete ptr; ptr=0; DataBase.sub();}
    > void operator=(T val){*ptr=val;}
    > SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs)
    > {
    > if(this!=&rhs)
    > {
    > delete ptr;
    > ptr(rhs.ptr);
    > }
    > else return this;
    > }
    > int status(){DataBase.status();}
    > T& operator*()const{return *ptr;}
    > T* operator->()const{return ptr;}
    > private:
    > SmrtPtrDB DataBase;
    > T* ptr;
    > };
    >
    > I'm very confused. Am I implementing this right? Thanks!!!!
    >


    This is our current counted pointer code:

    //
    //counted_ptr - simple reference counted pointer.
    //
    //The is a non-intrusive implementation that allocates an additional
    //int and pointer for every counted object.

    #ifndef __COUNTED_PTR_H
    #define __COUNTED_PTR_H

    template <class X> class counted_ptr
    {
    public:
    typedef X element_type;

    explicit counted_ptr(X* p = 0) // allocate a new counter
    : itsCounter(0) {if (p) itsCounter = new counter(p);}
    ~counted_ptr()
    {release();}
    counted_ptr(const counted_ptr& r) throw()
    {acquire(r.itsCounter);}
    counted_ptr& operator=(const counted_ptr& r)
    {
    if (this != &r) {
    release();
    acquire(r.itsCounter);
    }
    return *this;
    }

    template <class Y>
    counted_ptr(const counted_ptr<Y>& r) throw()
    {acquire(r.itsCounter);}
    template <class Y>
    counted_ptr& operator=(const counted_ptr<Y>& r)
    {
    if (this != &r) {
    release();
    acquire(r.itsCounter);
    }
    return *this;
    }

    X& operator*() const throw() {return *itsCounter->ptr;}
    X* operator->() const throw() {return itsCounter->ptr;}
    X* get() const throw() {return itsCounter ?
    itsCounter->ptr : 0;}
    bool unique() const throw()
    {return (itsCounter ? itsCounter->count == 1 : true);}

    private:

    struct counter {
    counter(X* p = 0, unsigned c = 1) : ptr(p), count(c) {}
    X* ptr;
    unsigned count;
    }* itsCounter;

    void acquire(counter* c) throw()
    { // increment the count
    itsCounter = c;
    if (c) ++c->count;
    }

    void release()
    { // decrement the count, delete if it is 0
    if (itsCounter) {
    if (--itsCounter->count == 0) {
    delete itsCounter->ptr;
    delete itsCounter;
    }
    itsCounter = 0;
    }
    }
    };

    #endif
     
    Jon Rea, Jul 10, 2006
    #2
    1. Advertising

  3. Jon Rea wrote:
    > Protoman wrote:
    >
    >> OK, this code compiles, links, and executes, but, how do I setup, like,
    >> a spinlock to query the DataBase object's status to let the SmrtPtr
    >> know that the object's been deleted?:


    It's very hard to understand what it is you're trying to do.

    Could you write a snippet of code that describes more about what you're
    trying to achieve ? More like a test case for your smart pointer ...
     
    Gianni Mariani, Jul 10, 2006
    #3
  4. Protoman

    Protoman Guest

    Gianni Mariani wrote:
    > Jon Rea wrote:
    > > Protoman wrote:
    > >
    > >> OK, this code compiles, links, and executes, but, how do I setup, like,
    > >> a spinlock to query the DataBase object's status to let the SmrtPtr
    > >> know that the object's been deleted?:

    >
    > It's very hard to understand what it is you're trying to do.
    >
    > Could you write a snippet of code that describes more about what you're
    > trying to achieve ? More like a test case for your smart pointer ...


    Actually here's the code; it achieves reference counting (I hope!!!)

    SmrtPtrDB.hpp

    #pragma once

    class SmrtPtrDB
    {
    public:
    SmrtPtrDB():num(1){}
    SmrtPtrDB(const SmrtPtrDB& rhs):num(rhs.num){}
    ~SmrtPtrDB(){}
    void add(){num++;}
    void sub(){num--;}
    int status(){return num;}
    private:
    int num;
    };

    SmrtPtr.hpp

    template<class T>
    class SmrtPtr
    {
    public:
    SmrtPtr<T>(T* obj=0):ptr(obj){}
    SmrtPtr<T>(const SmrtPtr<T>& rhs):ptr(rhs.ptr){DataBase.add();}
    ~SmrtPtr<T>()
    {
    DataBase.sub();
    if(DataBase.status()==0)
    {delete ptr;cout << "Deleted.";}
    else cout << "Out of scope. " << endl;
    }
    void operator=(T val){*ptr=val;}
    void operator=(T* val){ptr=val;}
    SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs)
    {
    if(this!=&rhs)
    {
    delete ptr;
    ptr(rhs.ptr);
    }
    else return this;
    }
    int status(){return DataBase.status();}
    T& operator*()const{return *ptr;}
    T* operator->()const{return ptr;}
    operator T*()const{return ptr;}
    private:
    SmrtPtrDB DataBase;
    T* ptr;
    };

    main.cpp

    #include <iostream>
    #include <cstdlib>
    #include "SmrtPtr.hpp"
    using namespace std;

    int main()
    {
    {
    SmrtPtr<int> ptr(new int);
    ptr=5;
    cout << *ptr << endl;
    {
    SmrtPtr<int> ptr2(ptr);
    ptr2=6;
    cout << *ptr << endl;
    cout << *ptr2 << endl;
    }
    cout << *ptr << endl;
    }
    system("PAUSE");
    return EXIT_SUCCESS;
    }


    It works apparently. Could you help me improve? Thanks!!!!
     
    Protoman, Jul 11, 2006
    #4
  5. Protoman schrieb:
    > Gianni Mariani wrote:
    >> Jon Rea wrote:
    >>> Protoman wrote:
    >>>
    >>>> OK, this code compiles, links, and executes, but, how do I setup, like,
    >>>> a spinlock to query the DataBase object's status to let the SmrtPtr
    >>>> know that the object's been deleted?:

    >> It's very hard to understand what it is you're trying to do.
    >>
    >> Could you write a snippet of code that describes more about what you're
    >> trying to achieve ? More like a test case for your smart pointer ...

    >
    > Actually here's the code; it achieves reference counting (I hope!!!)
    >

    [...]
    > system("PAUSE");
    > return EXIT_SUCCESS;
    > }
    >
    >
    > It works apparently. Could you help me improve? Thanks!!!!


    You could start by
    1) not using platform dependent things like system("PAUSE");
    2) using whitespace (indentation),
    3) describing, what you want to improve.

    Well, I don't think that it works, because your SmrtPtr<> class holds a
    SmrtPtrDB class (the ref counter) by value, while there should be only
    one counter per object, shared by the smart pointers. Also, you should
    handle operator=() by decrementing the count of the old object and
    incrementing the new one.

    --
    Thomas
     
    Thomas J. Gritzan, Jul 11, 2006
    #5
  6. Protoman

    Protoman Guest

    Thomas J. Gritzan wrote:
    > Protoman schrieb:
    > > Gianni Mariani wrote:
    > >> Jon Rea wrote:
    > >>> Protoman wrote:
    > >>>
    > >>>> OK, this code compiles, links, and executes, but, how do I setup, like,
    > >>>> a spinlock to query the DataBase object's status to let the SmrtPtr
    > >>>> know that the object's been deleted?:
    > >> It's very hard to understand what it is you're trying to do.
    > >>
    > >> Could you write a snippet of code that describes more about what you're
    > >> trying to achieve ? More like a test case for your smart pointer ...

    > >
    > > Actually here's the code; it achieves reference counting (I hope!!!)
    > >

    > [...]
    > > system("PAUSE");
    > > return EXIT_SUCCESS;
    > > }
    > >
    > >
    > > It works apparently. Could you help me improve? Thanks!!!!

    >
    > You could start by
    > 1) not using platform dependent things like system("PAUSE");
    > 2) using whitespace (indentation),
    > 3) describing, what you want to improve.
    >
    > Well, I don't think that it works, because your SmrtPtr<> class holds a
    > SmrtPtrDB class (the ref counter) by value, while there should be only
    > one counter per object, shared by the smart pointers. Also, you should
    > handle operator=() by decrementing the count of the old object and
    > incrementing the new one.
    >
    > --
    > Thomas


    Which one's the "old object", the one of the right side of operator=,
    or the left side? And do you mean this:

    SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs)
    {
    if(this!=&rhs)
    {
    this->DataBase.sub();
    delete ptr;
    ptr(rhs.ptr);
    rhs->DataBase.add();
    }
    else return this;
    }

    So, should DataBase be held by reference? And, system("PAUSE") is the
    only way I can see my output, otherwise, the console opens and closes
    before I get a chance to review. And I want to improve the handling of
    the reference count; I don't think my impl is very effecient or
    effective. And speed. And the handling of a null pointer, like if
    someone writes:

    SmrtPtr<int> ptr;

    ptr will be initialized to null with my ctor, so deferencing it will be
    illegal. I think it should throw an exception.
     
    Protoman, Jul 11, 2006
    #6
  7. Protoman

    Marcus Kwok Guest

    Protoman <> wrote:
    > And, system("PAUSE") is the
    > only way I can see my output, otherwise, the console opens and closes
    > before I get a chance to review.


    A standards-compliant way to achieve a similar effect is:

    std::cout << "\nPress <Enter> to continue...\n";
    std::string trash;
    std::getline(std::cin, trash);

    --
    Marcus Kwok
    Replace 'invalid' with 'net' to reply
     
    Marcus Kwok, Jul 12, 2006
    #7
  8. Protoman schrieb:
    > Thomas J. Gritzan wrote:
    >>> It works apparently. Could you help me improve? Thanks!!!!

    >> You could start by
    >> 1) not using platform dependent things like system("PAUSE");
    >> 2) using whitespace (indentation),
    >> 3) describing, what you want to improve.
    >>
    >> Well, I don't think that it works, because your SmrtPtr<> class holds a
    >> SmrtPtrDB class (the ref counter) by value, while there should be only
    >> one counter per object, shared by the smart pointers. Also, you should
    >> handle operator=() by decrementing the count of the old object and
    >> incrementing the new one.

    >
    > Which one's the "old object", the one of the right side of operator=,
    > or the left side?


    By assigning one smart pointer to another, you release the pointer on
    the left hand side and copy the pointer from the right hand side to the
    other.
    So you have to decrement the lhs counter and increment the rhs counter.
    But you must not delete the lhs pointer when the counter is not zero.

    > SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs)
    > {
    > if(this!=&rhs)
    > {
    > this->DataBase.sub();
    > delete ptr;


    Here you delete ptr. Don't do it when the counter is not zero.

    > ptr(rhs.ptr);
    > rhs->DataBase.add();


    rhs is const and DataBase is private. This line should not work.

    > }
    > else return this;
    > }
    >
    > So, should DataBase be held by reference?


    By pointer. Look at the implementation of other smart pointers, like
    shared_ptr from boost.

    Buy a good C++ book and read it.

    > And, system("PAUSE") is the
    > only way I can see my output, otherwise, the console opens and closes
    > before I get a chance to review.


    My IDE (Visual Studio) does this for me.

    --
    Thomas
     
    Thomas J. Gritzan, Jul 12, 2006
    #8
  9. Protoman

    Joe Seigh Guest

    Thomas J. Gritzan wrote:
    > By assigning one smart pointer to another, you release the pointer on
    > the left hand side and copy the pointer from the right hand side to the
    > other.
    > So you have to decrement the lhs counter and increment the rhs counter.
    > But you must not delete the lhs pointer when the counter is not zero.
    >
    >
    >>SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs)
    >>{
    >>if(this!=&rhs)
    >>{
    >>this->DataBase.sub();
    >>delete ptr;

    >
    >


    The standard way of doing this is to do a copy ctor on the source,
    swap with the destination, and let the dtor for the local copy,
    which is now the destination, run when it goes out of scope

    SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs) {
    SmrtPtr<T> temp(rhs);
    swap(temp);
    return *this;
    }

    You're safe in the case of source and destination being the same
    since the refcount will be incremented before it gets decremented.


    --
    Joe Seigh

    When you get lemons, you make lemonade.
    When you get hardware, you make software.
     
    Joe Seigh, Jul 12, 2006
    #9
  10. Protoman

    Protoman Guest

    Joe Seigh wrote:
    > Thomas J. Gritzan wrote:
    > > By assigning one smart pointer to another, you release the pointer on
    > > the left hand side and copy the pointer from the right hand side to the
    > > other.
    > > So you have to decrement the lhs counter and increment the rhs counter.
    > > But you must not delete the lhs pointer when the counter is not zero.
    > >
    > >
    > >>SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs)
    > >>{
    > >>if(this!=&rhs)
    > >>{
    > >>this->DataBase.sub();
    > >>delete ptr;

    > >
    > >

    >
    > The standard way of doing this is to do a copy ctor on the source,
    > swap with the destination, and let the dtor for the local copy,
    > which is now the destination, run when it goes out of scope
    >
    > SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs) {
    > SmrtPtr<T> temp(rhs);
    > swap(temp);
    > return *this;
    > }
    >
    > You're safe in the case of source and destination being the same
    > since the refcount will be incremented before it gets decremented.
    >
    >
    > --
    > Joe Seigh
    >
    > When you get lemons, you make lemonade.
    > When you get hardware, you make software.


    OK, here's the new SmrtPtr class:

    // COPYRIGHT CMDR DOUGLAS I. PEREIRA 07/10/06
    // ALL UNAUTHORIZED THIRD PARTY USE IS PROHIBITED
    // I HAVE WORKED VERY HARD AND HAVE SPENT HOURS OF
    // TIME DEVELOPING THIS. DO NOT COPY IT OR I WILL SUE YOU
    // ************************************************************
    #pragma once
    #include <iostream>
    #include "SmrtPtrDB.hpp"
    using namespace std;
    class NullPtr{};
    template<class T>
    class SmrtPtr
    {
    public:
    explicit SmrtPtr<T>(T* obj=0):ptr(obj), DataBase(new SmrtPtrDB){}
    SmrtPtr<T>(const SmrtPtr<T>& rhs):ptr(rhs.ptr), DataBase(new
    SmrtPtrDB(rhs.DataBase->status())){DataBase->add();}
    ~SmrtPtr<T>()
    {
    DataBase->sub();
    if(DataBase->status()==0)
    {delete ptr;cout << "Deleted." << endl;}
    else cout << "Out of scope. " << endl;
    }
    void operator=(T val){if(ptr==0)throw NullPtr();else *ptr=val;}
    void operator=(T* val){ptr=val;}
    SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs)
    {
    if(this!=&rhs)
    {
    SmrtPtr<T> temp(rhs);
    swap(temp);
    }
    else return *this;
    }
    bool operator==(const SmrtPtr<T>& rhs)const{if(ptr==rhs.ptr)return
    true;else return false;}
    bool operator!=(const SmrtPtr<T>& rhs)const{if(ptr!=rhs.ptr)return
    true;else return false;}
    bool operator<=(const SmrtPtr<T>& rhs)const{if(ptr<=rhs.ptr)return
    true;else return false;}
    bool operator>=(const SmrtPtr<T>& rhs)const{if(ptr>=rhs.ptr)return
    true;else return false;}
    int status(){return DataBase->status();}
    T& operator*()const{if(ptr==0)throw NullPtr();else return *ptr;}
    T* operator->()const{if(ptr==0)throw NullPtr();else return ptr;}
    operator T*()const{if(ptr==0)throw NullPtr();else return ptr;}
    private:
    void swap(SmrtPtr<T>& rhs){this=&rhs;}
    mutable SmrtPtrDB* DataBase;
    T* ptr;
    };

    Did I impl swap() correctly? Should DataBase be mutable?
     
    Protoman, Jul 13, 2006
    #10
  11. Protoman

    Joe Seigh Guest

    Protoman wrote:

    > SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs)
    > {
    > if(this!=&rhs)
    > {
    > SmrtPtr<T> temp(rhs);
    > swap(temp);
    > }
    > else return *this;
    > }


    Ah, I was going to mention not testing if the target
    and source were the same. One, it doesn't hurt anything
    if you don't test for it. Two, you increased your mainline
    path for something that occurs very rarely. When was the
    last time you saw a program assign a variable to itself?

    ....

    > private:
    > void swap(SmrtPtr<T>& rhs){this=&rhs;}
    > mutable SmrtPtrDB* DataBase;
    > T* ptr;
    > };
    >
    > Did I impl swap() correctly? Should DataBase be mutable?
    >


    swap should swap the contents of *this and rhs. Your
    basically faking things out so the destructor runs on
    the old target ptr.

    void swap(SmrtPtr<T>& rhs) {
    SmrtPtrDB* dbTemp = rhs.Database;
    T* ptrTemp = rhs.ptr;
    rhs.Database = Database;
    rhs.ptr = ptr;
    Database = dbTemp;
    ptr = ptrTemp;
    }

    Something like that.


    --
    Joe Seigh

    When you get lemons, you make lemonade.
    When you get hardware, you make software.
     
    Joe Seigh, Jul 13, 2006
    #11
  12. Protoman

    Protoman Guest

    Joe Seigh wrote:
    > Protoman wrote:
    >
    > > SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs)
    > > {
    > > if(this!=&rhs)
    > > {
    > > SmrtPtr<T> temp(rhs);
    > > swap(temp);
    > > }
    > > else return *this;
    > > }

    >
    > Ah, I was going to mention not testing if the target
    > and source were the same. One, it doesn't hurt anything
    > if you don't test for it. Two, you increased your mainline
    > path for something that occurs very rarely. When was the
    > last time you saw a program assign a variable to itself?
    >
    > ...
    >
    > > private:
    > > void swap(SmrtPtr<T>& rhs){this=&rhs;}
    > > mutable SmrtPtrDB* DataBase;
    > > T* ptr;
    > > };
    > >
    > > Did I impl swap() correctly? Should DataBase be mutable?
    > >

    >
    > swap should swap the contents of *this and rhs. Your
    > basically faking things out so the destructor runs on
    > the old target ptr.
    >
    > void swap(SmrtPtr<T>& rhs) {
    > SmrtPtrDB* dbTemp = rhs.Database;
    > T* ptrTemp = rhs.ptr;
    > rhs.Database = Database;
    > rhs.ptr = ptr;
    > Database = dbTemp;
    > ptr = ptrTemp;
    > }
    >
    > Something like that.
    >
    >
    > --
    > Joe Seigh
    >
    > When you get lemons, you make lemonade.
    > When you get hardware, you make software.


    OK, what if I wrote:

    void swap(SmrtPtr<T>& rhs)
    {
    *this=rhs;
    }

    No, wait...

    OK what about:

    void swap(SmrtPtr<T>& rhs)
    {
    std::swap(ptr,rhs.ptr);
    std::swap(DataBase,rhs.DataBase);
    }

    which is what I'm currently doing. And, self-assignment can happen in a
    very large program, where you have multiple references to the same
    thing, and the like. It's always good to test for it. And when I write
    this, the console opens and shuts:

    SmrtPtr<T> temp(ptr);
    ptr=ptr2;
    ptr2=temp;

    Why is that?
     
    Protoman, Jul 13, 2006
    #12
  13. Protoman schrieb:
    > OK, here's the new SmrtPtr class:
    >
    > // COPYRIGHT CMDR DOUGLAS I. PEREIRA 07/10/06
    > // ALL UNAUTHORIZED THIRD PARTY USE IS PROHIBITED
    > // I HAVE WORKED VERY HARD AND HAVE SPENT HOURS OF
    > // TIME DEVELOPING THIS. DO NOT COPY IT OR I WILL SUE YOU
    > // ************************************************************
    > #pragma once
    > #include <iostream>
    > #include "SmrtPtrDB.hpp"
    > using namespace std;
    > class NullPtr{};
    > template<class T>
    > class SmrtPtr
    > {
    > public:
    > explicit SmrtPtr<T>(T* obj=0):ptr(obj), DataBase(new SmrtPtrDB){}
    > SmrtPtr<T>(const SmrtPtr<T>& rhs):ptr(rhs.ptr), DataBase(new
    > SmrtPtrDB(rhs.DataBase->status())){DataBase->add();}


    The smartpointers should _share_ a common SmrtPtrDB, if they share a
    common object pointer. Here you create another copy of SmrtPtrDB in the
    copy constructor.

    > ~SmrtPtr<T>()
    > {
    > DataBase->sub();
    > if(DataBase->status()==0)
    > {delete ptr;cout << "Deleted." << endl;}
    > else cout << "Out of scope. " << endl;
    > }


    You don't delete the SmrtPtrDB.

    > void operator=(T val){if(ptr==0)throw NullPtr();else *ptr=val;}


    Remove this. The user should dereference the smart pointer to access its
    value.

    > void operator=(T* val){ptr=val;}


    Here you should release the old ptr/SmrtPtrDB and create a new pair.
    Something like this:

    void operator=(T* val)
    {
    SmrtPtr<T> temp(val);
    swap(temp);
    }

    > SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs)
    > {
    > if(this!=&rhs)
    > {
    > SmrtPtr<T> temp(rhs);
    > swap(temp);
    > }
    > else return *this;
    > }


    You return *this in the "else" but not in the "if". Why?

    > bool operator==(const SmrtPtr<T>& rhs)const{if(ptr==rhs.ptr)return
    > true;else return false;}
    > bool operator!=(const SmrtPtr<T>& rhs)const{if(ptr!=rhs.ptr)return
    > true;else return false;}
    > bool operator<=(const SmrtPtr<T>& rhs)const{if(ptr<=rhs.ptr)return
    > true;else return false;}
    > bool operator>=(const SmrtPtr<T>& rhs)const{if(ptr>=rhs.ptr)return
    > true;else return false;}
    > int status(){return DataBase->status();}
    > T& operator*()const{if(ptr==0)throw NullPtr();else return *ptr;}
    > T* operator->()const{if(ptr==0)throw NullPtr();else return ptr;}
    > operator T*()const{if(ptr==0)throw NullPtr();else return ptr;}
    > private:
    > void swap(SmrtPtr<T>& rhs){this=&rhs;}


    Better this way:
    > void swap(SmrtPtr<T>& rhs)
    > {
    > std::swap(ptr,rhs.ptr);
    > std::swap(DataBase,rhs.DataBase);
    > }


    > mutable SmrtPtrDB* DataBase;
    > T* ptr;
    > };


    Well, your operator= should return *this by reference, as every
    operator= does.

    Anyway, if you won't get your indentation right, I won't read your code
    anymore. It is horrible...

    --
    Thomas
     
    Thomas J. Gritzan, Jul 13, 2006
    #13
  14. Protoman

    Protoman Guest

    Thomas J. Gritzan wrote:
    > Protoman schrieb:
    > > OK, here's the new SmrtPtr class:
    > >
    > > // COPYRIGHT CMDR DOUGLAS I. PEREIRA 07/10/06
    > > // ALL UNAUTHORIZED THIRD PARTY USE IS PROHIBITED
    > > // I HAVE WORKED VERY HARD AND HAVE SPENT HOURS OF
    > > // TIME DEVELOPING THIS. DO NOT COPY IT OR I WILL SUE YOU
    > > // ************************************************************
    > > #pragma once
    > > #include <iostream>
    > > #include "SmrtPtrDB.hpp"
    > > using namespace std;
    > > class NullPtr{};
    > > template<class T>
    > > class SmrtPtr
    > > {
    > > public:
    > > explicit SmrtPtr<T>(T* obj=0):ptr(obj), DataBase(new SmrtPtrDB){}
    > > SmrtPtr<T>(const SmrtPtr<T>& rhs):ptr(rhs.ptr), DataBase(new
    > > SmrtPtrDB(rhs.DataBase->status())){DataBase->add();}

    >
    > The smartpointers should _share_ a common SmrtPtrDB, if they share a
    > common object pointer. Here you create another copy of SmrtPtrDB in the
    > copy constructor.
    >
    > > ~SmrtPtr<T>()
    > > {
    > > DataBase->sub();
    > > if(DataBase->status()==0)
    > > {delete ptr;cout << "Deleted." << endl;}
    > > else cout << "Out of scope. " << endl;
    > > }

    >
    > You don't delete the SmrtPtrDB.
    >
    > > void operator=(T val){if(ptr==0)throw NullPtr();else *ptr=val;}

    >
    > Remove this. The user should dereference the smart pointer to access its
    > value.
    >
    > > void operator=(T* val){ptr=val;}

    >
    > Here you should release the old ptr/SmrtPtrDB and create a new pair.
    > Something like this:
    >
    > void operator=(T* val)
    > {
    > SmrtPtr<T> temp(val);
    > swap(temp);
    > }
    >
    > > SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs)
    > > {
    > > if(this!=&rhs)
    > > {
    > > SmrtPtr<T> temp(rhs);
    > > swap(temp);
    > > }
    > > else return *this;
    > > }

    >
    > You return *this in the "else" but not in the "if". Why?
    >
    > > bool operator==(const SmrtPtr<T>& rhs)const{if(ptr==rhs.ptr)return
    > > true;else return false;}
    > > bool operator!=(const SmrtPtr<T>& rhs)const{if(ptr!=rhs.ptr)return
    > > true;else return false;}
    > > bool operator<=(const SmrtPtr<T>& rhs)const{if(ptr<=rhs.ptr)return
    > > true;else return false;}
    > > bool operator>=(const SmrtPtr<T>& rhs)const{if(ptr>=rhs.ptr)return
    > > true;else return false;}
    > > int status(){return DataBase->status();}
    > > T& operator*()const{if(ptr==0)throw NullPtr();else return *ptr;}
    > > T* operator->()const{if(ptr==0)throw NullPtr();else return ptr;}
    > > operator T*()const{if(ptr==0)throw NullPtr();else return ptr;}
    > > private:
    > > void swap(SmrtPtr<T>& rhs){this=&rhs;}

    >
    > Better this way:
    > > void swap(SmrtPtr<T>& rhs)
    > > {
    > > std::swap(ptr,rhs.ptr);
    > > std::swap(DataBase,rhs.DataBase);
    > > }

    >
    > > mutable SmrtPtrDB* DataBase;
    > > T* ptr;
    > > };

    >
    > Well, your operator= should return *this by reference, as every
    > operator= does.
    >
    > Anyway, if you won't get your indentation right, I won't read your code
    > anymore. It is horrible...
    >
    > --
    > Thomas


    For swap(), how about *this=rhs? Will that work?

    Here's the stuff I fixed:

    ~SmrtPtr<T>()
    {
    DataBase->sub();
    if(DataBase->status()==0)
    {
    delete ptr;
    delete DataBase;
    cout << "Deleted." << endl;
    }
    else
    {
    delete DataBase;
    cout << "Out of scope. " << endl;
    }
    }

    void operator=(T* val)
    {
    SmrtPtr<T> temp(val);
    swap(temp);
    }

    SmrtPtr<T>& operator=(const SmrtPtr<T>& rhs)
    {
    SmrtPtr<T> temp(rhs);
    swap(temp);
    return *this;
    }

    void swap(const SmrtPtr<T>& rhs){*this=rhs;}


    Sorry about my indenting, it looks normal on my screen.
     
    Protoman, Jul 13, 2006
    #14
  15. Protoman schrieb:
    > For swap(), how about *this=rhs? Will that work?


    No, swap() _swaps_ the contents, *this=rhs assigns the rhs to the lhs
    (lhs gets overridden).

    > Here's the stuff I fixed:
    >
    > ~SmrtPtr<T>()
    > {
    > DataBase->sub();
    > if(DataBase->status()==0)
    > {
    > delete ptr;
    > delete DataBase;
    > cout << "Deleted." << endl;
    > }
    > else
    > {
    > delete DataBase;


    Really, think about what you are doing here. Ask yourself this questions:

    - Who is the owner of the object pointed to by ptr? Who may delete it
    and when?
    - Who owns DataBase, who deletes it?

    > Sorry about my indenting, it looks normal on my screen.


    Try another newsreader, or try spaces instead of tabs.

    --
    Thomas
     
    Thomas J. Gritzan, Jul 15, 2006
    #15
  16. Protoman

    Protoman Guest

    Thomas J. Gritzan wrote:
    > Protoman schrieb:
    > > For swap(), how about *this=rhs? Will that work?

    >
    > No, swap() _swaps_ the contents, *this=rhs assigns the rhs to the lhs
    > (lhs gets overridden).
    >
    > > Here's the stuff I fixed:
    > >
    > > ~SmrtPtr<T>()
    > > {
    > > DataBase->sub();
    > > if(DataBase->status()==0)
    > > {
    > > delete ptr;
    > > delete DataBase;
    > > cout << "Deleted." << endl;
    > > }
    > > else
    > > {
    > > delete DataBase;

    >
    > Really, think about what you are doing here. Ask yourself this questions:
    >
    > - Who is the owner of the object pointed to by ptr? Who may delete it
    > and when?
    > - Who owns DataBase, who deletes it?
    >
    > > Sorry about my indenting, it looks normal on my screen.

    >
    > Try another newsreader, or try spaces instead of tabs.
    >
    > --
    > Thomas


    So, how do I fix it?
     
    Protoman, Jul 15, 2006
    #16
  17. Protoman

    Protoman Guest

    Protoman wrote:
    > Thomas J. Gritzan wrote:
    > > Protoman schrieb:
    > > > For swap(), how about *this=rhs? Will that work?

    > >
    > > No, swap() _swaps_ the contents, *this=rhs assigns the rhs to the lhs
    > > (lhs gets overridden).
    > >
    > > > Here's the stuff I fixed:
    > > >
    > > > ~SmrtPtr<T>()
    > > > {
    > > > DataBase->sub();
    > > > if(DataBase->status()==0)
    > > > {
    > > > delete ptr;
    > > > delete DataBase;
    > > > cout << "Deleted." << endl;
    > > > }
    > > > else
    > > > {
    > > > delete DataBase;

    > >
    > > Really, think about what you are doing here. Ask yourself this questions:
    > >
    > > - Who is the owner of the object pointed to by ptr? Who may delete it
    > > and when?
    > > - Who owns DataBase, who deletes it?
    > >
    > > > Sorry about my indenting, it looks normal on my screen.

    > >
    > > Try another newsreader, or try spaces instead of tabs.
    > >
    > > --
    > > Thomas

    >
    > So, how do I fix it?


    OK, I fixed the above, but, now I'm wondering: is SmrtPtr<T> safe to
    use in a container, such as a linked list? Is it thread safe?
     
    Protoman, Jul 17, 2006
    #17
  18. Protoman schrieb:
    [ SmrtPtr<> template class ]
    > OK, I fixed the above, but, now I'm wondering: is SmrtPtr<T> safe to
    > use in a container, such as a linked list? Is it thread safe?
    >


    Depends on the container, if it can be used with it.

    For the standard container classes, the values have to have
    value-semantics. If your smart pointer works as it should, then its safe
    to use in on of them.

    For thread safety: Don't ask here, since C++ doesn't know about threads
    (you should know this already)

    --
    Thomas
     
    Thomas J. Gritzan, Jul 18, 2006
    #18
  19. Protoman

    Protoman Guest

    Thomas J. Gritzan wrote:
    > Protoman schrieb:
    > [ SmrtPtr<> template class ]
    > > OK, I fixed the above, but, now I'm wondering: is SmrtPtr<T> safe to
    > > use in a container, such as a linked list? Is it thread safe?
    > >

    >
    > Depends on the container, if it can be used with it.
    >
    > For the standard container classes, the values have to have
    > value-semantics. If your smart pointer works as it should, then its safe
    > to use in on of them.
    >
    > For thread safety: Don't ask here, since C++ doesn't know about threads
    > (you should know this already)
    >
    > --
    > Thomas


    OK, but how do I access this:

    class Data
    {
    public:
    Data():plus(new long double),mult(new long double),pow(new long
    double){}
    SmrtPtr<long double> plus;
    SmrtPtr<long double> mult;
    SmrtPtr<long double> pow;
    };

    SmrtPtr<Data> tuple(new Data);
    //how do I access the members of tuple?
     
    Protoman, Jul 18, 2006
    #19
  20. Protoman schrieb:
    > Thomas J. Gritzan wrote:
    >> Protoman schrieb:
    >> [ SmrtPtr<> template class ]
    >>> OK, I fixed the above, but, now I'm wondering: is SmrtPtr<T> safe to
    >>> use in a container, such as a linked list? Is it thread safe?
    >>>

    >> Depends on the container, if it can be used with it.
    >>
    >> For the standard container classes, the values have to have
    >> value-semantics. If your smart pointer works as it should, then its safe
    >> to use in on of them.
    >>
    >> For thread safety: Don't ask here, since C++ doesn't know about threads
    >> (you should know this already)
    >>
    >> --
    >> Thomas

    >
    > OK, but how do I access this:
    >
    > class Data
    > {
    > public:
    > Data():plus(new long double),mult(new long double),pow(new long
    > double){}
    > SmrtPtr<long double> plus;
    > SmrtPtr<long double> mult;
    > SmrtPtr<long double> pow;
    > };
    >
    > SmrtPtr<Data> tuple(new Data);
    > //how do I access the members of tuple?


    Smart pointers usually imitate the syntax of normal pointers.

    tuple->plus
    tuple->mult
    tuple->pow

    or:

    (*tuple).plus
    ....

    --
    Thomas
     
    Thomas J. Gritzan, Jul 19, 2006
    #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. ash
    Replies:
    1
    Views:
    436
    SenderX
    Oct 24, 2003
  2. Kalle Rutanen

    Reference counting in C++

    Kalle Rutanen, May 7, 2005, in forum: C++
    Replies:
    0
    Views:
    577
    Kalle Rutanen
    May 7, 2005
  3. Tony Johansson

    reference counting

    Tony Johansson, May 22, 2005, in forum: C++
    Replies:
    4
    Views:
    379
    Andrew Koenig
    May 23, 2005
  4. mathieu
    Replies:
    8
    Views:
    523
    Juha Nieminen
    Aug 31, 2008
  5. edwardfredriks

    counting up instead of counting down

    edwardfredriks, Sep 6, 2005, in forum: Javascript
    Replies:
    6
    Views:
    224
    Dr John Stockton
    Sep 7, 2005
Loading...

Share This Page