Implementing Smart Pointer - Dynamic Allocation with templates

Question

I'm in the process of writing a smart pointer countedptr and I've hit a speed bump. The basic function of countedptr is to work like any other smart pointer and also have a count of how many pointers are pointing to a single object. So far, the code is:

[SOLVED]

#include "std_lib_facilities.h"

template <class T>
class counted_ptr{
private:
    T* pointer;
    int* count;

public:
    counted_ptr(T* p = 0, int* c = new int(1)) : pointer(p), count(c) {}    // default constructor
    explicit counted_ptr(const counted_ptr& p) : pointer(p.pointer), count(p.count) { ++*count; } // copy constructor
    ~counted_ptr() { --*count; delete pointer; }

    counted_ptr& operator=(const counted_ptr& p)
    {
        pointer = p.pointer;
        count = p.count;
        ++*count;
        return *this;
    }
    T* operator->() const{ return pointer; }
    T& operator*() const { return *pointer; }

    int Get_count() const { return *count; }
};


int main()
{
    counted_ptr<double> one;
    counted_ptr<double>two(one);
    int a = one.Get_count();
    cout << a << endl;
}

When I try to do something like

one->pointer = new double(5);

then I get a compiler error saying "request for member 'pointer' in '*(&one)->counted_ptr::operator->with T = double' which is of non-class type double".

I considered making a function to do this, and while I could make a function to allocate an array of T's, I can't think of a way of making one for allocating actual objects. Any help is appreciated, thanks.

Answer 1

Did you, perhaps, mean "one.pointer=new double(5);"? Writing "one->pointer=new double(5);" invokes counted_ptr<double>::operator->. That is, it is approximately equivalent to:

double *tmp = one.operator->(); // returns one.pointer
tmp->pointer = new double(5);

But a double pointer isn't a structure, and so it doesn't have a pointer member.

Answer 2

Try it,

one=new double(5);

Answer 3

Old Solution

What about another assignment operator?

counted_ptr& counted_ptr::operator=(T* p)
{
    if (! --*count) { delete count; }
    pointer = p;
    count = new int(1);
    return *this;
}

...

one = new double(5);

Also, your destructor always deletes a shared pointer, which is probably what caused *one to be a random nomber. Perhaps you want something like:

counted_ptr::~counted_ptr() { if (! --*count) { delete pointer; delete count; } }

New Solution

As you want repointing a counted_ptr (eg one = new double(5)) to update all related counted_ptrs, place both the pointer and the count in a helper class, and have your pointer class hold a pointer to the helper class (you might already be headed down this path). You could go two ways in filling out this design:

1. Make the helper class a simple struct (and a private inner class) and place all the logic in the outer class methods

2. Make counted_ptr the helper class. counted_ptr maintains a reference count but doesn't automatically update the count; it's not a smart pointer, it only responds to release and retain messages. If you're at all familiar with Objective-C, this is basically its traditional memory management (autoreleasing aside). counted_ptr may or may not delete itself when the reference count reaches 0 (another potential difference from Obj-C). counted_ptrs shouldn't be copyable. The intent is that for any plain pointer, there should be at most one counted_ptr.

   Create a smart_ptr class that has a pointer to a counted_ptr, which is shared among smart_ptr instances that are supposed to hold the same plain pointer. smart_ptr is responsible for automatically updating the count by sending its counted_ptr release and retain methods.

   counted_ptr may or may not be a private inner class of shared_ptr.

Here's an interface for option two. Since you're doing this as an exercise, I'll let you fill out the method definitions. Potential implementations would be similar to what's already been posted except that you don't need a copy constructor and copy assignment operator for counted_ptr, counted_ptr::~counted_ptr doesn't call counted_ptr::release (that's smart_ptr::~smart_ptr's job) and counted_ptr::release might not free counted_ptr::_pointer (you might leave that up to the destructor).

// counted_ptr owns its pointer an will free it when appropriate.
template <typename T>
class counted_ptr {
private:
    T *_pointer;
    size_t _count;

    // Make copying illegal
    explicit counted_ptr(const counted_ptr&);
    counted_ptr& operator=(const counted_ptr<T>& p);

public:
    counted_ptr(T* p=0, size_t c=1);
    ~counted_ptr();

    void retain();        // increase reference count.
    bool release();       // decrease reference count. Return true iff count is 0
    void reassign(T *p);  // point to something else.
    size_t count() const;

    counted_ptr& operator=(T* p);

    T& operator*() const;
    T* operator->() const;
};

template <typename T>
class smart_ptr {
private:
    counted_ptr<T> *_shared;
    void release();  // release the shared pointer
    void retain();   // retain the shared pointer

public:
    smart_ptr(T* p=0, int c=1);   // make a smart_ptr that points to p
    explicit smart_ptr(counted_ptr<T>& p); // make a smart_ptr that shares p
    explicit smart_ptr(smart_ptr& p); // copy constructor
    ~smart_ptr();

    // note: a smart_ptr's brethren are the smart_ptrs that share a counted_ptr.
    smart_ptr& operator=(smart_ptr& p); /* Join p's brethren. Doesn't alter pre-call
        * brethren. p is non-const because this->_shared can't be const. */
    smart_ptr& operator=(counted_ptr<T>& p);  /* Share p. Doesn't alter brethren. 
        * p is non-const because *this isn't const. */
    smart_ptr& operator=(T* p); // repoint this pointer. Alters brethren

    size_t count() const; // reference count

    T& operator*() const;  // delegate these to _shared
    T* operator->() const;

};

Hopefully, the only ambiguous points above are the intentional ones.

Answer 4

(Sorry, newbie here, and can't leave comments). What Adatapost added, "one=new double(5);" should work. One other change needed, though: the reference counting needs a little help.

...
~counted_ptr() {
    --*count; 
    // deallocate objects whose last reference is gone.
    if (!*count) 
    {   
        delete pointer;
        delete count;
    }
}

counted_ptr& operator=(const counted_ptr& p)
{
    // be careful to accommodate self assignment
    ++*p.count;

    // may lose a reference here
    --*count;
    if (!*count)
    {
        delete pointer;
        delete count;
    }

    count=p.count;
    pointer=p.pointer;
    return *this;
}

Of course, there's some code repetition here. It might make sense to refactor that code into its own function, e.g.

private:
    /** remove our reference */
    void release()
    {
        --*count;
        if (!*count)
        {
            delete pointer;
            delete count;
        }
    }

Answer 5

Unless you are not doing this for academic reasons, you might want to use consider using the use_count() member of boost::shared_ptr. It's not entirely efficient, but it does work and you're better off using something well tested, mature, and thread safe. If you are doing this for learning purposes, be sure to check out the treatment of Reference Counting and Smart Pointers in More Effective C++.

Answer 6

You need to decrement the count and possibly delete the pointer to the old value in operator = before you overwrite it. You also need 'delete count' everywhere you have 'delete pointer' to avoid leaking memory

Answer 7

Since the immediate problem has already been solved, I want to offer something more long term:

As you continue to develop this code, you'll definitely want to offer it up for full review by experienced programmers, whether here or elsewhere. There were a few obvious problems with your code as you posted it, though outis has helped correct them. But even once your code all compiles and seems to work in your own tests, there may be tests and situations which you haven't yet learned to think about. Smart pointers can easily have subtle problems that don't show up until very specific situations. So you'll want others to look over your code to find anything which you may have missed.

Please don't take this as any kind of insult towards your current code. I'm just offering this as friendly advice to ensure you learn the most you can out of this project.