I made a reference-counting smart pointer class. My aim is to make a "minimal" but "general purpose" smart pointer class with proper documentation. This is basically for educational purposes.

I would like to have comments regarding exception handling, code clarity, comments, ease of use of API, etc. And, also comment if the code is of "industrial level".

#ifndef SMARTPTR_HPP
#define SMARTPTR_HPP

#include <algorithm>

namespace smartptrnamespace
{

//
// Basic reference counting smart pointer (no overloaded Bool magic, etc).
// Ownership of memory on heap is shared amongst objects of this
// class (shared ownership) i.e., no new object of T type is created
// by this class.
// Objects of this class are:
//  1. nothrow-copy-assignable
//  2. nothrow-copy-constructible
//  3. nothrow-destructible, if T is nothrow-destructible
//  4. suitable for storage in a STL container like List, Deque etc.
// Strong exception guarantee
//
// Sample Usage:
//  SmartPtr<T> sPtr1 (new T);  // sPtr1 refers to object of T type
//  SmartPtr<T> sPtr2;          // sPtr2 refers to no object
//  if (!sPtr2.isAssigned ())   // SmartPtr::isAssigned() returns false if no reference is being held
//      cout << "sPtr2 is not holding any reference" << endl;
//  sPtr2 = new T;              // make a new oject of T and pass ownership to sPtr
//
template<class T>
class SmartPtr
{
    public:

        // create a new object which is not
        // refering to any object on heap.
        // no-throw guarantee
        explicit SmartPtr ()
            :   m_pT (NULL),
                m_pRefCount (NULL)
        {
        }

        // new object will point to a memory location on heap given by 'pObj'
        // Strong exception guarantee
        explicit SmartPtr (T *pObj)
            :   m_pT (pObj),
                m_pRefCount (NULL)
        {
            try
            {
                m_pRefCount = new int;
            }
            catch (...)
            {
                checkedDelete (m_pT);
                throw;
            }

            *m_pRefCount = 1;
        }

        // new object will refer to same memory on heap as 'rObj'
        // no-throw guarantee
        SmartPtr (const SmartPtr<T> &rObj)
            :   m_pT(rObj.m_pT), 
                m_pRefCount(rObj.m_pRefCount) 
        { 
            if (m_pRefCount != NULL)
                (*m_pRefCount)++;
        }


        // make 'rObj' and 'this' will refer to same object on heap
        // no-throw guarantee
        SmartPtr& operator= (const SmartPtr<T> &rObj)
        {
            // uses copy-and-swap idiom
            this_type(rObj).swap(*this);
            return *this;
        }

        // assign this smart pointer to another object on heap
        // Strong exception guarantee
        SmartPtr& operator= (T *pTObj)
        {
            // try and setup memory for reference counter
            int *pNewRefCount;
            try                             
            {
               pNewRefCount = new int;
            }
            catch (...)
            {
                delete pTObj;
                throw;
            }

            // stop referring to previous object
            updateCountAndTriggerDelete ();

            // start referring to new object
            m_pRefCount = pNewRefCount;
            *m_pRefCount = 1;

            m_pT = pTObj;

            return *this;
        }

        // no-throw guarantee
        ~SmartPtr ()
        {
            updateCountAndTriggerDelete ();
        }

        // returns true if this object is holding a reference
        // to an object on heap
        // no-throw guarantee
        bool isAssigned()
        {
            return !(m_pT == NULL);
        }

        // no-throw guarantee
        T* operator->()
        {
            return m_pT;
        }

        // no-throw guarantee
        T& operator*()
        {
            return *m_pT;
        }

    private:

        // make sure we dont delete a incomplete type pointer
        // no-throw guarantee
        template <class S>
        void checkedDelete (S* pSObj)
        {
            typedef char type_must_be_complete[ sizeof(S)? 1: -1 ];
            (void) sizeof(type_must_be_complete);
            delete pSObj;     
        }

        // count the references and delete it if this is last reference
        // no-throw guarantee
        void updateCountAndTriggerDelete ()
        {
            if (m_pRefCount != NULL)
            {                  
                (*m_pRefCount)--;

                // if this is last reference delete the memory
                if (*m_pRefCount == 0)
                {
                    checkedDelete (m_pRefCount);
                    checkedDelete (m_pT);
                }
            }
        }

        // swap the pointer values of 'rObj' with values of 'this'
        // no-throw guarantee
        void swap (SmartPtr<T> &rObj)
        {
            std::swap (m_pT, rObj.m_pT);
            std::swap (m_pRefCount, rObj.m_pRefCount);
        }

        // pointer to memory location of object
        T *m_pT;

        // pointer to memory location where 'reference' count of
        // object pointed to by m_pT is kept
        int *m_pRefCount;

        typedef SmartPtr<T> this_type;
};

} // namespace smartptrnamespace

#endif // SMARTPTR_HPP
  • 1
    I'm not sure... but it seems your copy operators don't handle self assignment (cf. Effective C++ item 11) – Offirmo Apr 18 '13 at 8:40
  • 2
    I would also add an exception throw if someone try to dereference a NULL smart pointer. In my opinion it would be useful. – Offirmo Apr 18 '13 at 8:42
  • 2
    I would rename updateCountAndTriggerDelete to decreaseCountAndTriggerDelete – Offirmo Apr 18 '13 at 8:45
  • 1
    Also, your smart pointer is not thread-safe. – Offirmo Apr 18 '13 at 8:46
  • 2
    You can add const in a lot of places : params, isAssigned(). That would make your code more robust and able to work in more situation. – Offirmo Apr 18 '13 at 8:54

A few non-expert comments:

  • I would prefer the private vars to be at the top of the class - to avoid the immediate need to scroll to the bottom.

  • Is there and advantage in making m_pRefCount a pointer to int instead of a simple int?

  • Why would you need to use checkedDelete to delete a pointer to int that you created yourself? If you don't use checkedDelete for your local int, then the function can take template type T not a new class S. Is there not a better way to check for an incomplete type? And (showing my ignorance) is it necessary?

  • 4
    It has to be a pointer or otherwise how would multiple instances of the class access the same int? SharedPtr b = a; b needs to increment a's counter too. In this case, it's simple to just go inside of a and increment it, but in other cases (such as when a falls out of scope, but b doesn't) simple manipulation like that isn't possible. – Corbin Apr 18 '13 at 22:22
  • in c++, we usually put public members at the top. – Abyx Apr 19 '13 at 8:24
  • 3
    @Abyx: Don't agree with that. I would also have put the private member variables at the top to aid in readability. I would put the private member functions at the bottom. – Martin York Apr 19 '13 at 17:14
  • @LokiAstari better readability for people who use you class? – Abyx Apr 19 '13 at 18:09
  • 3
    @Abyx: For people that maintain the class. The most important users. – Martin York Apr 19 '13 at 20:28

1-- Why not nullptr in place of NULL ?

2-- Personally I like if (m_pRefCount) more that if (m_pRefCount != NULL)

3-- In your SmartPtr& operator= (T *pTObj) you delete pTObj; but in your explicit SmartPtr (T *pObj) you checkedDelete (m_pT); Why this inconsistence?

4-- Implementing manually SmartPtr& operator= (const SmartPtr<T> &rObj) could be a litter more efficient, but I find very good your use of the copy-swap: it allow not to duplicate code. Why not use it for;

    SmartPtr& operator= (T *pTObj)
    {
        If (pTObj== m_pT) return *this;     // this may be a litter better.
       // uses copy-and-swap idiom
        this_type(pTObj).swap(*this);    // temporary SmartPtr get deleted, and m_pRefCount decreased (after swap: for the old value).
        return *this;
    }

In this case the only place where updateCountAndTriggerDelete (); will be called is the destructor, and you could move it entirely code there, with seems more natural for me. That also mean that you will don’t need to zeroes any deleted pointer.

~SmartPtr ()
{
       if ( ! m_pRefCount) return;

        if ( 0== --(*m_pRefCount )  )     //     :-)     if this is last reference delete the memory
        {
            delete m_pRefCount;
            checkedDelete (m_pT);
        }
}

5-- Also (from your answer to comments) this SmartPtr& operator= (T *pTObj) is not used during initialization:

T *oldPtr = new T();

SmartPtr<T> sPtr = oldPtr;   // will not compile. Here you have: SmartPtr sPtr ( oldPtr);    but you declared this constructor explicit.
The use of this will be:
SmartPtr<T> sPtr;
sPtr = oldPtr;   
sPtr = oldPtr;   // second time you have a problem if not insert the If (pTObj== m_pT). But this may be OK.

The construction T a=b; is a short for T a(b); and the operator=() is only used to assign a value to an already constructed object, and not to construct a new object and initialize it.

6-- As others said, add const where possible.

  • obviously it's C++03 code, that's why no nullptr – Abyx Apr 19 '13 at 18:10
  1. API should resemble API of existing shared pointers, like std::(tr1::)shared_ptr. They have widely known well-designed API and there is no need to invent something new without a good reason.

  2. Pointer should have an operator bool, instead of isAssigned and it should be const.
    In C++ it will be explicit operator bool() const, in C++03 - safe-bool idiom.

  3. Those try-catch blocks are awful. You should write unique_ptr first, and use it to temporally own the object:

    explicit SmartPtr (T *pObj) : m_pT(pObj), m_pRefCount()
    {
        unique_ptr<T> temp(pObj);
        m_pRefCount = new int(1);
        temp.release();
    }
    
  4. Assignment operator can take argument by value:

    void operator= (SmartPtr<T> rObj) { swap(rObj); }
    
  5. Both copy ctor an assignment operator should be templates to allow code like

    struct Derived : Base {...};        
    SmartPtr<Derived> d = ...;
    SmartPtr<Base> b = d;
    
  6. operator= (T *pTObj) should use SmartPtr (T *pObj) ctor and dtor

    void operator= (T *pTObj) { SmartPtr<T>(pTObj).swap(*this); }
    
  7. SmartPtr<T>(NULL) is broken.

  8. You can write typedef SmartPtr this_type; (without <T>)

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.