0

I am trying to understand how to give back memory if one class creates another class.

I have

 Clas A;

Then another class that allocate memory for Class A:

class B{
    private:
        A* data;
    public:
        // Allocating new memory
        B (){
                  A* data =  new A();

        //giving memory back
        ~B(){
                   delete data; };
};

And when I execute the code in main function it just crashes. What am doing wrong? I am a bit lost here. Thanks.

Pasha
  • 169
  • 1
  • 1
  • 11

2 Answers2

5

Get rid of the redundant A* in the constructor. What this does is create a new, local variable with the same name as your class member. So the real B::data never gets anything assigned to it, and when you try to delete it, things blow up. To add insult to injury, the new A() you assign to the local data will be leaked (well; it would be leaked if the program didn't crash).

class B{
    private:
        A* data;
    public:
        // Allocating new memory
        B (){
           data =  new A();
        }

        //giving memory back
        ~B(){
           delete data;
        }
};

That solves the immediate problem, but as juanchopanza noted in the comments, you will still run into problems if you try to copy this object.

dlf
  • 9,045
  • 4
  • 32
  • 58
  • 2
    Also, use smart pointers instead of manually implementing new and delete. The RAII pattern saves you a lot of grief. – tillaert Apr 29 '14 at 22:26
  • 3
    This class is still broken. You run into trouble if you copy or assign a `B`. – juanchopanza Apr 29 '14 at 22:31
  • A legitimate point. I was trying to address the immediate, obvious problem, but that one could certainly lead to more trouble down the road. – dlf Apr 29 '14 at 22:49
  • Ohhh...it was that easy. I guess my mind just bogged down. Thanks a lot guys. – Pasha Apr 29 '14 at 23:00
  • Glad it helped. Also make sure to either hide the copy constructor & assingment operator or provide working implementations if you haven't already (I am assuming the code you posted does not represent the entire class). – dlf Apr 29 '14 at 23:01
  • Yeah. It was just a small part where I got stuck. Couldn't realize I did such a stupid mistake. Thanks again. My code works now. – Pasha Apr 29 '14 at 23:07
0

Here's the RAII / Rule of Zero route (assuming your compiler supports C++11)

class A {};

class B {
private:
    std::unique_ptr<A> data;

public:
    B() : data(new A) {
    }
};

The RAII handle in this case a unique_ptr will take care of deallocation for you. Implementing it this means the compiler defined copy constructor, copy assignment operator, move constructor, move assignment operator and destructor will all work fine right out the box.

PeterSW
  • 4,921
  • 1
  • 24
  • 35