Implementing a copy on write String class using reference counting in C++

Question

I have tried to implement a copy on write string class with value semantics and reference counting.

I wanted to support indexing, so I have overloaded the operator[] and to differentiate between the l-value and r-value usage it is returning a proxy object.

#include <iostream>
#include <string.h>
class proxy;

//Reference counting class.Also holds the string.
class RC
{
    public:
        RC(const char * str);
        ~RC();
    private:
        char* str;
        int count;

    friend class mystring;
    friend class proxy;

};


class mystring
{
    public:
        mystring(const char*  str);
        mystring& operator=(const mystring& str);
        mystring(const mystring& str);
        ~mystring();

        void print() const;
        proxy operator[](int n);

    private:
        RC *pRC;

    friend class proxy;
};
class proxy
{
    public:
        proxy(mystring* r,int n);
        proxy& operator=(const char& c);
        proxy& operator=(const proxy& c);
        operator char() const;

    private:
        int n;
        mystring* ptr;
};


RC::RC(const char * str1)
:str(new char[strlen(str1)+1]),count(1)
{ 
    strcpy(str,str1);
}

RC::~RC()
{
    if(--count == 0)
        delete [] str;
}

proxy mystring::operator[](int n)
{
    return proxy(this,n);
}

mystring::mystring(const char*  str)
:pRC(new RC(str))
{
}
mystring& mystring::operator=(const mystring& rhs)
{
    if(this != &rhs)
    {
        if(--pRC->count == 0)
            delete pRC;
        pRC = rhs.pRC;
        ++pRC->count;
    }
    return *this;
}
mystring::mystring(const mystring& rhs)
{
    pRC = rhs.pRC;
    ++pRC->count;
}

mystring::~mystring()
{
    if(--pRC->count == 0)
        delete pRC;
}
void mystring::print() const
{
    std::cout << pRC->str << ":" << pRC->count << '\n';
}


proxy::operator char() const
{
    return ptr->pRC->str[n];
}

proxy::proxy(mystring* r,int n) 
:ptr(r),n(n)
{}

proxy& proxy::operator=(const char& c)
{

    if(c != ptr->pRC->str[n])
    {
        char cpy[strlen(ptr->pRC->str)+1];
        strcpy(cpy,ptr->pRC->str);
        ptr->pRC->~RC();
        cpy[n] = c;
        ptr->pRC = new RC(cpy);
    }

    return *this;
}


proxy& proxy::operator=(const proxy& rhs)
{
    if(this != &rhs)
    {
        *this = (char)rhs;

    }   

    return *this;
}

Can you please help me improve the code?

Answer 1

Code simplification

mystring acts essentially as a smart pointer. It's already implemented in the standard library. You can use the std::shared_ptr class. This way, mystring can just have an std::shared_ptr<std::string> as a member. The default copy constructor, assignment operator and the desturctor will work properly.

Use of non-standard features

char cpy[strlen(ptr->pRC->str)+1]

variable sized arrays are not a part of the standard C++. Don't use them. If you need it, you should allocate it dynamically.

If you use the std::shared_ptr<std::string> as I said above, it'll be even simpler as std::string has a proper copy constructor.

Index

It's conventional to use size_t, not int as an index type (at least that's how an std::string works).

Answer 2

My suggestions:

Make RC a private, nested class

RC is an implementation detail of mystring. It's best to make it a private, nested class.

RC does not follow the rule of three

RC allocates memory in its constructor and deallocates the memory in the destructor. All such classes should follow The Rule of Three.

You can get away with disregarding the rule of three if RC is a private, nested class. Even then, there is a possibility that you will run into problems if the implementations of any of the member functions of mystring is not mindful of that fact.

Provide operator[] function in RC

You had to make proxy a friend of RC in order to be able to access strc. Had you provided an operator[] function, that would have been necessary. Given the choices, I would recommend providing operator[] function(s) in RC.

Fix the memory leak in RC

You have:

RC::~RC()
{
   if(--count == 0)
      delete [] str;
}

However, the destructor of RC is called only when count is equal to 0 in couple of places.

In mystring::operator=(const mystring& rhs):

  if(--pRC->count == 0)
     delete pRC;

In mystring::~mystring:

  if(--pRC->count == 0)
     delete pRC;

That means, you need to change the destructor of RC to be:

RC::~RC()
{
   // No need for this. count is already 0
   // if(--count == 0)
      delete [] str;
}

Use a less cryptic name instead of RC

Examples:

1. mystring::ReferenceCountedData
2. mystring::Data
3. mystring:Implementation

I would go with mystring::Data. The fact that it is reference counted is evident from the goal of mystring.

Provide a const version of the operator[] function

The current version of operator[] is a non-const function. If a function has a const object of type mystring, it won't be able to use the operator[] function get access the contents of the object. You should provide a const version of the operator[] function.

char operator[](size_t index) const
{
   return pRC->str[index];
}

If you would like to use a proxy for that function as well, you will need to use another class called const_proxy. That is analogous to the use of iterator and const_iterator in the container classes in the standard library.

const_proxy operator[](size_t index) const
{
   return const_proxy(this, index);
}

I'll leave the implementation of const_proxy as a detail.

Avoid mystring::print

I would recommend using an overload of operator<<. That makes mystring usable with all kinds of ostream objects. It also makes the task of outputting objects of the class more idiomatic.

std::ostream& operator<<(std::ostream& out, mystring const& s)
{
   return (out << pRC->str << ":" << pRC->count);
}

Now, you can use:

mystring obj("My Test String");
std::cout << obj << std::endl;