Need Help Deallocating Memory in a Class Destructor

Question

I receive the following error when I run my test program:

The error goes away if I comment out "delete name," but I can't seem to figure out why. Hopefully someone can give me insight into a solution.

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

class Shape
{
protected:
    string* name = new string;
public:
    Shape(string str)
    {
        *name = str; 
    }
    ~Shape()
    {
        delete name;  //Why does my program run if I comment this out?
        name = NULL;
    }

    virtual double getArea() const
        { return 0; }
    void setName(string str)
        { *name = str; }
    string getName() const
        { return *name; }
};

class Circle : public Shape
{
private:
    double radius;
public:
    Circle(string str = "", double r = 0) : Shape(str)
        { radius = r; }
    void setRadius(double r)
        { radius = r; }
    double getRadius() const
        { return radius; }
    virtual double getArea() const
        { return radius * radius * 3.14; }

    Circle operator= (const Circle &);
    Circle operator+ (const Circle &);
};

Circle Circle::operator= (const Circle &right)
{
    radius = right.radius;
    return *this;
}

Circle Circle::operator+ (const Circle &right)
{
    Circle tempCircle;
    tempCircle.radius = radius + right.radius;
    return tempCircle;
}

int main()
{
    Circle c1("c1",5);
    Circle c2("c2",10);
    Circle c3;

    c3 = c2 + c1;

    cout << "c1" << c1.getRadius() << endl;
    cout << "c2" << c2.getRadius() << endl;
    cout << "c3" << c3.getRadius() << endl;
    return 0;
}

I'm just using a string pointer to test my understanding of correctly using a copy constructor with a pointer. — Eric Larson, May 01 '16 at 02:06
@Matimio your code does not contain any user-defined copy constructors, that's the whole problem — M.M, May 01 '16 at 03:05

score 2 · Answer 1 · edited May 23 '17 at 10:28

2

You are using the compiler defined copy constructor when you use:

c3 = c2 + c1;

That is a problem when you have member data that are allocated from the heap and deallocated in the destructor.

Add proper copy constructors to your classes.

See The Rule of Three for further details.

You can obviate the need for user defined copy constructors and copy assignment operators by changing

string* name = new string;

to

string name;

and changing the rest of your code accordinlgy.

edited May 23 '17 at 10:28

Community

1
1

answered May 01 '16 at 02:06

R Sahu

204,454
14
159
270

Doesn't my overloaded operator functions fix that? Is there something wrong with my implementation of them? – Eric Larson May 01 '16 at 02:09
@Matimio Where do you have an `operator=` for the `Shape` class that handles performing a deep copy of the `std::string*`? What you end up doing is getting 2 or more different class instances pointing the same `std::string*` which means things go bad after `delete` is performed by the first instance. – James Adkison May 01 '16 at 02:10
I'm just using this program to study for my upcoming test. I was just seeing if I could properly use a copy constructor with a pointer. – Eric Larson May 01 '16 at 02:10
1

Your overload functions are only partially correct. They don't handle the base class part at all. – R Sahu May 01 '16 at 02:10
"Doesn't my overloaded operator functions fix that?" - Just try your class with `Shape(const Shape&) = delete;` and see what the compiler has to say about that ;-) Same with the assignment operator. – VolkerK May 01 '16 at 02:12
@RSahu I wasn't aware I had to create a copy constructor for my base Shape class if I was purely using Circle class. – Eric Larson May 01 '16 at 02:12
@Matimio The base class needs to have it's state properly copied too. – James Adkison May 01 '16 at 02:13
Why doesn't making Shape an abstract base class fix this problem since I can't have an instance of Shape at that point? – Eric Larson May 01 '16 at 02:18
@Matimio, that requires a long explanation that is not appropriate as a comment. It will be better ask to a different question, if you can create a [Minimal, Complete, and Verifiable example](http://stackoverflow.com/help/mcve). – R Sahu May 01 '16 at 03:00
@M.M, indeed. Thanks. – R Sahu May 01 '16 at 03:04
@VolkerK deleting `Shape` assignment operator does not break compilation, the code does not call that function anyway. But it would be a good idea to implement that function of course, in case a later change to the code does try to assign a Shape – M.M May 01 '16 at 03:06
OK. The actual instance of calling broken copy constructor is in the return of `operator+` – M.M May 01 '16 at 03:07
@VolkerK that code deletes copy-constructor. I was talking about deleting assignment operator. – M.M May 01 '16 at 11:12
deleting the Circle assignment operator would break compilation as well. But yes, my point was Shape's copy ctor. – VolkerK May 01 '16 at 11:46

Need Help Deallocating Memory in a Class Destructor

1 Answers1