C++ A self-defined class String assigning it to a C-style String

Question

In my assignment I created a class called String. Here's the template:

class String
{
    // the raw string buffer
    char *m_pString;

    // the capacity of this buffer
    int m_capacity;     // note: this is not the length of the string!
public:
    // the default constructor
    String(int size = 1);
    // a constructor from a char-string
    String(const char* src, int size = 1);
    // a copy constructor
    String(const String& src, int size = 1);
    // the assignment operator
    String& operator=(const String& src);
    // the destructor
    ~String();


    // a method to resize the buffer
    virtual void resize(int newsize);
    // method to return the current capacity of the buffer
    int capacity() { return m_capacity; }


    // method to return the length of the string
    int         length()                    const;
    // true if the string is empty
    bool        empty()                  const;
    // return a substring of the string
    String      substr(int index, int len)const;

    // operators functions
    // add a char to the end of the string
    String&     operator+=  (char c);
    // append a string to the end of this one
    String&     operator+=  (const String& s2);
    // return string A + string B
    String      operator+   (const String& s2) const;
    // cast the string as a C-style string
    operator    const char* ()                  const;
    // true if the string is valid, false if empty
    operator bool()                  const;
    // true if string 1 == string 2
    bool        operator==(const String& s2)const;



};

My question is what happens if we write something like this:

String a;
a = "hello";

The 'a' is of the class String(an object) that I defined above while "hello" is a c-style string, I don't get how this can work, because these are in my main function from the assignment and I am trying to make it work.

This is the definition for the assignment operator which I think is where my problem is:

    String& String::operator=(const String& other)
{
    int a = strlen(other.m_pString);
    m_pString = nullptr;
    m_pString = new char[a];
    strcpy(m_pString, other.m_pString);
    m_capacity = a;
    return *this;
}

Can anyone tell me how I should edit the assignment operator function to make it work?

Answer 1

Your class already has an operator= that accepts a String as input, and a non-explicit constructor that accepts a const char* as input. So, a = "hello" will invoke an implicit conversion using a temp object, similar to this:

String a;
a.operator=(String("hello"));

The real problem is that your operator= is leaking memory, and is not allocating new memory correctly. You are not freeing m_pString before reassigning it, and you are not allocating enough memory for a null terminator, which strlen() and strcpy() require.

String& String::operator=(const String& other) {
    int a = strlen(other.m_pString);
    m_pString = nullptr; // <-- leak here!
    m_pString = new char[a]; // <-- 'a' is too small!
    strcpy(m_pString, other.m_pString);
    m_capacity = a;
    return *this;
}

You need to do something more like this instead:

String& String::operator=(const String& other)
{
    if (&other != this)
    {
        int a = strlen(other.m_pString) + 1;
        delete[] m_pString;
        m_pString = new char[a];
        strcpy(m_pString, other.m_pString);
        m_capacity = a;
    }
    return *this;
}

Or this, which is safer:

String& String::operator=(const String& other)
{
    if (&other != this)
    {
        String temp(other);
        std::swap(m_pString, temp.m_pString);
        std::swap(m_capacity, temp.m_capacity);
    }
    return *this;
}

Of course, these assume that your other methods (constructors, destructor, substr(), etc) are managing allocated memory correctly as well.