Reference Counting Template

This is a discussion on Reference Counting Template within the C++ Programming forums, part of the General Programming Boards category; I'm having a problem with a template class I wrote that will allow me to have reference counters for object ...

  1. #1
    Registered User
    Join Date
    Oct 2009
    Posts
    48

    [SOLVED] Reference Counting Template

    I'm having a problem with a template class I wrote that will allow me to have reference counters for object pointers. I have objects contained in multiple lists/queues and I need to prevent the object from being deleted every time it is removed from a list.

    This is what I have
    Code:
    template<class T>
    class SmartPtr
    {
        public:
            explicit SmartPtr(T* obj)
                : ptr(obj) { obj->addRef(); }
            explicit SmartPtr() {}
            SmartPtr(const SmartPtr<T>& orig)
            {
                ptr = orig.ptr;
                ptr->addRef();
            }
            ~SmartPtr()
            {
                ptr->removeRef();
                if (ptr->ref_count() == 0) delete ptr;
            }
            T& operator*()      { return *ptr;  }
            T* operator->()     { return ptr;    }
            SmartPtr<T>& operator=(SmartPtr<T> right)
            {
                if (right.ptr)
                    right.ptr->addRef();
    
                ptr = right.ptr;
    
                return *this;
            }
        private:
            T* ptr;
    };
    Basically, I want the shared functionality of Boost's shared_ptr but I'd like to implement it.

    My problem is that I am getting a segfault when I use the above code and it's not near anywhere where I use the SmartPtr so it is difficult to pinpoint what is wrong.
    Last edited by BdON003; 11-25-2009 at 02:19 PM.

  2. #2
    The larch
    Join Date
    May 2006
    Posts
    3,573
    It looks like the assignment operator is incorrect. It shouldn't modify the right-hand value. And it should decrement the reference count on the left-hand value. (Basically, I think the copy-and-swap idiom should do the trick again.)

    Next, this SmartPtr allows NULL pointers, but the destructor doesn't check for NULL. Neither does the first constructor check whether the pointer is non-NULL. And the default constructor leaves ptr uninitialized.
    Last edited by anon; 11-25-2009 at 01:09 PM.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  3. #3
    Registered User
    Join Date
    Oct 2009
    Posts
    48
    Quote Originally Posted by anon View Post
    It looks like the assignment operator is incorrect. It shouldn't modify the right-hand value. And it should decrement the reference count on the left-hand value. (Basically, I think the copy-and-swap idiom should do the trick again.)
    Ok, so I've changed the assignment operator to
    Code:
    SmartPtr<T>& operator=(SmartPtr<T> right)
            {
                if (this != &right)
                {  swap(right, *this);  }
    
                this->ptr->addRef();
    
                return *this;
            }
            void swap(SmartPtr<T> &a, SmartPtr<T> &b)
            {
                SmartPtr<T> temp(a);
                a = b;
                b = temp;
            }
    But that code doesn't work because I end up in an infinite recursive loop.

  4. #4
    Registered User
    Join Date
    Oct 2009
    Posts
    48
    Quote Originally Posted by anon View Post
    Next, this SmartPtr allows NULL pointers, but the destructor doesn't check for NULL. Neither does the first constructor check whether the pointer is non-NULL. And the default constructor leaves ptr uninitialized.
    I think this takes care of those points, but I'm still doing the copy-and-swap wrong so I could use a pointer about that.

    Code:
    template<class T>
    class SmartPtr
    {
        public:
            explicit SmartPtr(T& obj)
                : ptr(&obj)
                {
                    if (obj != NULL)
                    obj.addRef();
                }
            explicit SmartPtr(T* obj)
                : ptr(obj)
                {
                    if (*obj != NULL)
                    obj->addRef();
                }
            explicit SmartPtr() {ptr = NULL;}
            SmartPtr(const SmartPtr<T>& orig)
            {
                ptr = orig.ptr;
                if (*ptr != NULL)
                    ptr->addRef();
            }
            ~SmartPtr()
            {
                if (*ptr != NULL
                {
                    ptr->removeRef();
                    if (ptr->ref_count() == 0) delete ptr;
                }
            }
            T& operator*()      { return *ptr;  }
            T* operator->()     { return ptr;    }
            SmartPtr<T>& operator=(SmartPtr<T> right)
            {
                if (this != &right)
                {  swap(right, *this);  }
    
                this->ptr->addRef();
    
                return *this;
            }
            void swap(SmartPtr<T> &a, SmartPtr<T> &b)
            {
                SmartPtr<T> temp(a);
                a = b;
                b = temp;
            }
        private:
            T* ptr;
    };

  5. #5
    The larch
    Join Date
    May 2006
    Posts
    3,573
    First, the swap method should not be implemented in terms of assignment operator.

    I suppose, all it takes to swap two SmartPtr's is to swap the pointers they hold.

    Code:
    //member function
    void swap(SmartPtr<T>& other)
    {
        std::swap(ptr, other.ptr);
    }
    You might also implement a free swap function (in terms of member swap), which will hopefully get picked up, for example, when a standard algorithm needs to swap your SmartPtr instances.

    And now assignment operator would be implemented like this:

    Code:
    SmartPtr<T>& operator=(const SmartPtr<T>& right)
    {
        //this takes care of incrementing ref-count on right
        SmartPtr<T> temp(right);
    
        //swap contents of left-hand instance and the local copy
        swap(temp);
    
        return *this;
        //destructor of temp takes care of decrementing ref-count on the pointer previously held by *this
    }
    (It would also be possible to accept right by value: then temp won't be needed, since a copy will be automatically made.)
    Last edited by anon; 11-25-2009 at 01:58 PM.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  6. #6
    Registered User
    Join Date
    Oct 2009
    Posts
    48
    Thanks anon!

  7. #7
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,308
    Welcome to the "copy and swap" idiom!

    It's the best thing since sliced bread...
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Undefined Reference Compiling Error
    By AlakaAlaki in forum C++ Programming
    Replies: 1
    Last Post: 06-27-2008, 11:45 AM
  2. Replies: 6
    Last Post: 06-05-2007, 12:38 AM
  3. Gnarly Linking Error HELP!!!!
    By brooksbp in forum C++ Programming
    Replies: 8
    Last Post: 05-04-2007, 01:00 AM
  4. question about .net to 6.0 change causing errors
    By jverkoey in forum C++ Programming
    Replies: 17
    Last Post: 03-23-2004, 09:45 AM
  5. Template specialization
    By CornedBee in forum C++ Programming
    Replies: 2
    Last Post: 11-25-2003, 01:02 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21