Copy Constructor question

Question

I'm trying to understand this copy constructor problem. The question I have pertains to the destructor after the program exits. It seems the variable char* title is not destructed and I think this maybe wrong, thanks

Another question is why the assignment operator isn't called when the object x is equal to x2. I'm using g++ with codeblocks.

#include <iostream>

using namespace std;  

class myClass
{
    private:

        int x;
        int *p;
        char* title;

    public:

        myClass(int tx, int* temp, char* newtitle)
        {
            x = tx;
            cout << "int constructor called with value x = " << x << endl;

            p = new int;
            p = temp;

            title = new char [strlen(newtitle)+1];
            strcpy(title, newtitle);
        }
        myClass(const myClass& mc)
        {
            cout << "copy constructor called with value = " << mc.x << endl;
            x = mc.x;
            p = new int;
            *p = *mc.p;

            title = new char [strlen(mc.title)+1];
            strcpy(title, mc.title);
        }
        myClass & operator = (const myClass & that)
        {
            cout << "assignment called" << endl;

            if(this != &that)
            {
                x = that.x;
            }
            return *this;
        }
        ~myClass()
        {
            if (p)
            {
                cout<<"deleting p"<<endl;
                delete p;
            }
            else if(title)
            {
                cout<<"deleting title"<<endl;
                delete[] title;
            }
        }
};

int main()
{
    int pointee = 10;
    int* p = &pointee;
    myClass x(3,p,"hello");
    //myClass y = myClass(3,p);
    myClass x2 = x;
    return 0;
}

Answer 1

You have a variety of problems that I was able to see, both in your actual code and in your general approach.

First of all, the char* title isn't deleted because you don't delete it. This is probably a logical error:

    if (p)
    {
        cout<<"deleting p"<<endl;
        delete p;
    }
    else if(title)
    {
        cout<<"deleting title"<<endl;
        delete[] title;
    }

You probably don't need the else. Why did you put it there?

Next, you are leaking an int, here:

    p = new int;
    p = temp;

The int you just new-ed gets overwritten by the passed-in value temp.

Later, you try to delete this pointer in the destructor. But since you are deleting a pointer to an automatic variable, you will hose the heap. This is also a logical error. Solution: don't do this: p = temp;

Ultimately however, your approach is questionable on multiple levels.

1. Why do you dynamically allocate ints in the first place? Just have an int member of the class. Don't use dynamic allocation (eg new and delete) unless you really have to.
2. Don't dynamically allocate strings using char*s. Instead, use std::string from #include <string>
3. If you really need dynamic allocation, don't use raw pointers. Use smart pointers instead. C++ comes with one built-in, std::auto_ptr from #include <memory.h> but there are many other options, often better choices, in other libraries. A popular one here is Boost's smart pointers.

Answer 2

Your destructor deletes p if p is non-NULL; and it deletes title if it's non-NULL and p is NULL.

But both your constructor and your copy constructor create new p and new title all the time. So you need to check and delete both all the time.

Answer 3

Try

*p = *temp;

instead of

 p = temp;

and in the destructor

if(title)

instead of

else if(title)