Constructors taking references in C++

Question

I'm trying to create constructor taking reference to an object. After creating object using reference I need to prints field values of both objects. Then I must delete first object, and once again show values of fields of both objects. My class Person looks like this :

class Person {   
    char* name;
    int age;
public:

    Person(){
        int size=0;
        cout << "Give length of char*" << endl;
        cin >> size;
        name = new char[size];      
        age = 0;
    }

    ~Person(){
        cout << "Destroying resources" << endl;
        delete[] name;
        delete age;
    }  

void init(char* n, int a) {
    name = n;
    age = a;
} 
}; 

Here's my implementation (with the use of function show() ). My professor said that if this task is written correctly it will return an error.

#include <iostream>
using namespace std;

class Person {   
    char* name;
    int age;
public:

    Person(){
        int size=0;
        cout << "Give length of char*" << endl;
        cin >> size;
        name = new char[size];      
        age = 0;
    }

    Person(const Person& p){
        name = p.name;
        age = p.age;
    }

    ~Person(){
        cout << "Destroying resources" << endl;
        delete[] name;
        delete age;
    }  

void init(char* n, int a) {
    name = n;
    age = a;
}


void show(char* n, int a){
    cout << "Name: " << name << "," << "age: " << age << "," << endl; 
}

}; 


int main(void) {
    Person *p = new Person;  
    p->init("Mary", 25);

    p->show();

    Person &p = pRef;
    pRef->name = "Tom";
    pRef->age = 18;

    Person *p2 = new Person(pRef);

    p->show();
    p2->show();

    system("PAUSE");
    return 0;
}

Answer 1

The problem with your copy constructor is that it merely assigns p.name:

name = p.name // Now this and p hold a pointer to the same memory

Since both this and p now hold a pointer to the same memory location, whichever one destructs first will free the memory while the second one will be holding a pointer to a non-existent object. Subsequently using that pointer or deleting it will result in undefined behavior. The solution is to allocate a new array for name and copy the contents of p.name into that array, so that the memory isn't shared.

Likewise, your init function overwrites name, ignoring the fact that memory has been allocating (it is a memory leak) and also ignoring the fact that the string will later be destructed (even though the caller probably expects to own and free that string, itself). Also, I should point out that your show function takes a parameter "n", but uses "name", instead. Your show function probably shouldn't take any parameters (indeed, the way you call it implies that it doesn't), given that all the needed fields are already present in your class (or maybe you intended for that to be a freestanding function that takes the fields of the class?). You should take another look at your code for additional errors.

Answer 2

First of all, try to compile your code (it is usually a good idea to compile code before posting it on SO.) It contains several errors and compiler will show them. Second part will be to change the following:

p->init("Mary", 25);

to

{
  std::string mary("Mary");
  p->init(mary.c_str(), 25);
}

It should give you an error at runtime and it will give you a chance to find a problem in your implementation.