CODEWITHSUNDEEP
c++memory-managementmemory-leaks
user99545
user99545

Reputation: 1173

Dealing with memory leaks in class new and delete operators C++

I enjoy using the operators new and delete in C++ a lot but often have a problem calling delete later on in the program's code.

For example, in the following code:

class Foo {
public:
    string *ace;
    Foo();
    ~Foo();
};
Foo::Foo() {
    ace = new string;
}
Foo::~Foo() {
    delete ace;
}
void UI::ButtonPressed() { //Happens when an event is triggered
    Foo *foo = new Foo;
    ui->label = ace; //Set some text on the GUI
    delete foo; //Calls the destructor, deleting "ace" and removing it from the GUI window
}

I can declare a new string but when I delete it, it removes the value from the GUI form because that string has now been deleted.

Is there a way for me to delete this allocated string somehow later on?

I don't want to declare it as a global variable and then delete it on the last line of the program's source code. I could just never call delete but from what I have been taught that's bad and results in memory leaks.

Upvotes: 2

Views: 944

Answers (4)

celtschk
celtschk

Reputation: 19731

Well, there's a lot to say of your code. Some things have already been said, e.g. that you should make string a normal member so the allocation/deallcoation issue goes away completely (that's a general rule for C++ programs: If you don't absolutely have to use dynamic allocation, then don't, period). Also, using an appropriate smart pointer would do the memory management for you (also a general rule in C++: Don't manage the dynamic allocations yourself unless you really have to).

However let's pretend that you have to use dynamic allocation, and you have to use raw pointers and direct new and delete here. Then another important rule comes in (which actually isn't a C++ specific rule, but a general OO rule): Don't make the member public. Make it a private member, and offer a public member function for setting it. That public member function then can properly delete the old object before assigning the pointer to the new one. Note that as soon as you assigned the pointer, unless you've stored the old value elsewhere, the old value is lost forever, and if the object has not been deleted up to then, you can't delete it later.

You also want to consider whether it is really a good idea to take ownership of an object passed to you by pointer (and assigning to a pointer member which has a delete in the destructor is a ― not very obvious ― way to pass ownership). This complicates the object lifetime management because you have to remember whether you passed a certain object to an ownership-claiming object (this is not an issue if you have a strict policy of always passing to ownership-claiming objects, though). As usual, smart pointers may help here; however you may consider whether it is a better option to make a copy of the passed object (for std::string it definitely is, but then, here it's better to have a direct member anyway, as mentioned above).

So here's a full list of rules, where earlier rules take precedence to later unless there's a good reason not to use it:

  1. Don't use dynamical allocation.
  2. Manage your dynamical allocation with smart pointers.
  3. Use new only in constructors and delete only in the corresponding destructor.
  4. Always have the new and delete for a specific pointer in member functions of the same class. (Actually the previous rule is a special case of this one, but a special case which should be preferred to the general one.)

Upvotes: 1

justin
justin

Reputation: 104728

Here's a more idiomatic C++ program:

class Foo {
public:
    std::string ace;

    Foo() : ace() {
      // nothing to do here. ace knows how to create itself…
    }

    // and copy itself…
    Foo(const Foo& other) : ace(other.ace) {}

     // and clean up after itself…
    ~Foo() {
    }

    // and copy/assign itself…
    Foo& operator=(const Foo& other) {
      this->ace = other.ace;
      return *this;
    }
};


void UI::ButtonPressed() {
  // `new` is not needed here, either!
  Foo foo;
  ui->label = foo.ace; //Set some text on the GUI
  // `delete` is not needed here
}

If you really need to call new, always use an appropriate smart pointer -- Writing delete is banished from modern C++ ;)

Upvotes: 0

vvnraman
vvnraman

Reputation: 1343

If you are using std::string for both ace and ui->label then you don't have to worry about the memory for foo->ace being deleted once the foo object goes out of scope.

A copy of the Right-Hand argument is made available to ui->label on an = (assignment operation). You can read more about it on the C++ std::string reference page for string::operator=.

Also, such problems can be avoided in full by using smart pointers, such as the ones provided by the boost library. Read this great post on stackoverflow on this subject to get a better understanding.

Upvotes: 1

Jørgen Fogh
Jørgen Fogh

Reputation: 7656

You should read about the RAII pattern. It is one of the most important concepts to know for a C++ programmer.

The basic idea is that the lifetime of a resource (a new'ed object, an HTTP connection, etc.) is tied to the lifetime of an object. This is necessary in order to write exception safe code.

In your case, the UI widget would make a copy of the object and free it in its own destructor. The calling code could then free its copy right away (in another destructor).

Upvotes: 1

Related Questions