c++ deep copy constructor

Question

this is my code:

#include<iostream>
#include<cstring>
using namespace std;

class String
{
private:
    char *s;
    int size;
public:
    String(const char *str = NULL); // constructor
    String& operator=(String &c){
        size = strlen(c.s);
        s = new char[size+1];
        strcpy(s, c.s);
    }
    ~String() { delete [] s; }// destructor
    void print() { cout << s << endl; }
    void change(const char *); // Function to change
};

String::String(const char *str)
{
    size = strlen(str);
    s = new char[size+1];
    strcpy(s, str);
}

void String::change(const char *str)
{
    delete [] s;
    size = strlen(str);
    s = new char[size+1];
    strcpy(s, str);
}

int main()
{
    String str1("learnc++");
    String str2 = str1;

    str1.print(); // what is printed ?
    str2.print();

    str2.change("learnjava");

    str1.print(); // what is printed now ?
    str2.print();
    return 0;
}

it can be compiled, and the result is:

learnc++ 
learnc++ 
learnjava 
learnjava

in addition to that,there is:

*** Error in `./code': double free or corruption (fasttop): 0x0000000000f7f010 ***

BTY, if I delete "delete [] s;" in String::change,the result just becomes:

learnc++
learnc++
learnc++
learnjava

and no error appers, and why ? the code is from geek foe feeks, I have changes some strings, and the code can be run in its IDE, but in my ubuntu 14.04, it cannot.

Answer 1

Your class is not following the Rule of Three because it is missing a proper copy constructor.

String str2 = str1; is just syntax sugar for String str2(str1);, so it uses the copy constructor, not your operator= (which has a memory leak, BTW).

Since you did not provide a copy constructor, the compiler provided one for you, but it does not make a deep copy of the char* data. It just copies the pointer itself, which causes the behaviors you are seeing.

A proper implementation would look more like this:

#include <iostream>
#include <cstring>
#include <algorithm>
using namespace std;

class String
{
private:
    char *s;
    int size;
public:
    String(const char *str = NULL);
    String(const String &src);
    ~String();
    String& operator=(const String &rhs);
    void print() const;
};

String::String(const char *str)
{
    size = strlen(str);
    s = new char[size+1];
    strcpy(s, str);
}

String::String(const String &src)
{
    size = src.size;
    s = new char[size+1];
    strcpy(s, src.s);
}

String::~String()
{
    delete [] s;
} 

void String::print() const
{
    cout << s << endl;
}

String& String::operator=(const String &rhs)
{
    if (&rhs != this)
    {
        String tmp(rhs);
        swap(s, tmp.s);
        swap(size, tmp.size);
    }
    return *this;
}

int main()
{
    String str1("learnc++");
    String str2 = str1;

    str1.print();
    str2.print();

    //str2.change("learnjava");
    str2 = "learnjava";

    str1.print();
    str2.print();

    return 0;
}

If you are using C++11 or later, you can use this implementation instead, which follows the Rule of Five by adding move semantics:

#include <iostream>
#include <cstring>
#include <utility>
using namespace std;

class String
{
private:
    char *s;
    int size;
public:
    String(const char *str = nullptr);
    String(const String &src);
    String(String &&src);
    ~String();
    String& operator=(String rhs);
    void print() const;
};

String::String(const char *str)
{
    size = strlen(str);
    s = new char[size+1];
    strcpy(s, str);
}

String::String(const String &src)
{
    size = src.size;
    s = new char[size+1];
    strcpy(s, src.s);
}

String::String(String &&src)
{
    size = src.size;
    s = src.s;
    src.s = nullptr;
    src.size = 0;
}

String::~String()
{
    delete [] s;
} 

void String::print() const
{
    cout << s << endl;
}

String& String::operator=(String rhs)
{
    swap(s, rhs.s);
    swap(size, rhs.size);
    return *this;
}