Dynamic allocation of classes

Question

I have a question about object array allocation, the problem is this: I have a class called person :

class Person {
    char* name;
    int ID;


public:
    Person(char* name = NULL, int ID = 0) :name(name), ID(ID) {};
}

and I'm trying to make an array of persons like so:

Person *pArr = new Person[size];

and after that I extract the data from a file (char* for string and int for ID) and I use for loop and the constructors to place the persons inside the loop like so:

for (int j = 0; j < size3; j++) {
pArr[j] = Person(Name, id);
}

after I finish my program I want to use the Destructor and free the allocated string that's stored inside the char* name, but when I add the Destructor it gets triggered right after the loops ends, every Person that's being created is destroyed immediately, I know its possible to make an array of pointers and allocated the person aswell, but in this assignment I'm suppose to do it like this, is there a right way to do it without triggering the destructor right away?

Answer 1

On this line:

pArr[j] = Person(Name, id);

You are creating a temporary Person object, and then assigning it to the pArr[j] object. Person has no explicit copy assignment operator defined, so the compiler auto-generates one for you, which simply performs a member-by-member copy of values from one object to another.

When the temporary goes out of scope on the ;, it is automatically destroyed. The compiler-generated assignment operator copied the name pointer as-is to the pArr[j] object, so if Person has a destructor that frees its name, the pArr[j] object will be left with a dangling name pointer to freed memory.

Your Person class is not following the Rule of Three:

The rule of three (also known as the Law of The Big Three or The Big Three) is a rule of thumb in C++ (prior to C++11) that claims that if a class defines one (or more) of the following it should probably explicitly define all three:

• destructor
• copy constructor
• copy assignment operator

Since you want Person to have a destructor that frees name, it also needs a copy constructor and a copy assignment operator so it can make copies of name for the destructor to free, eg:

class Person
{
    char* name;
    int id;

public:
    Person(const char* Name = NULL, int ID = 0)
        : name(new char[std::strlen(Name)+1]), id(ID)
    {
        std::strcpy(name, Name);
    }

    Person(const Person &src)
        : name(new char[std::strlen(src.name)+1]), id(src.id)
    {
        std::strcpy(name, src.name);
    }

    ~Person()
    {
        delete[] name;
    }

    Person& operator=(const Person &rhs)
    {
        if (&rhs != this)
        {
            Person tmp(rhs);

            //std::swap(tmp.name, name);
            char *p = tmp.name;
            tmp.name = name;
            name = p;

            id = tmp.id;
        }
        return *this;
    }
};

Now, back on this line:

pArr[j] = Person(Name, id);

The temporary will be copied to pArr[j] safely and properly. Just note that if you dynamically allocated Name beforehand, you will now have to free it afterwards, since Person makes its own internal copy.

---

If you can change char* to std::string, then you don't have to worry about the compiler making default copies, since std::string is compliant with the Rule of Three and will be copied properly:

class Person
{
    std::string name;
    int id;

public:
    Person(const std::string &name = std::string(), int ID = 0)
        : name(name), id(ID)
    {
    }

    // no explicit copy constructor, destructor, or
    // assignment operator is needed, as compiler-generated
    // defaults will suffice...
};