Old New and Delete Question

Started by
9 comments, last by frob 4 years, 10 months ago

With the following code may I put :  delete button; in class frame's destructor so that it will delete the button properly on exit of the program.  The reason I ask is I have many of these to do.

HEADER:

class frame

{

public:

wxButton * button

.

.

.

}

 

CPP:

frame::frame()

{

button = new wxButton( …);

.

.

.

}

 

It is different than :  wxButton * button new wxButton(...); 

 

Thank you,

Joshua

Advertisement

The formatting is a bit messed up, making your question hard to read.  I can see four different ways to interpret it.

 

What I think you are asking is if you can use member initializer lists or a similar syntax for a destructor. No, you cannot. Constructors do quite a lot of special work under the hood, and it potentially is quite slow.  Initializer lists reduce the complexity of some of that work, in addition to providing a potentially more simple syntax.

 

But you might be asking different questions, I'm not sure because of your formatting.

If you are asking about calling new during constructors, it is possibly legal depending on details, and it is bad form so you should avoid it.

Doing this is potentially very bad in code:

MyClass::MyClass() : mSomeInts (new int[32]) { ... }

The biggest problem is from exceptions. If new throws an exception it leaves a partly-constructed object and the destructor isn't called. There are plenty of unexpected ways that type of allocation can leak resources or crash the program. There is a way to wrap try/catch blocks around constructors but it isn't pretty and it can be confusing.

In general it is bad form for default constructors to do any work at all other than to make an empty object. Containers and classes representing larger resources should generally be set to an empty state. You might provide additional functions that allocate resources, and provide additional constructors that do the work if you must, but they should not be the default constructor.  Constructors are called all the time, including for things like making large arrays and the compiler creating temporary objects. Creating an empty object is generally nearly instant, just initialize counters to zero and pointers to nullptr.  

 

Or, you might also be asking about the nature of cleaning up resources.  Most objects have both allocation and release functions, those aren't part of the constructor but instead functions that trigger allocating resources and functions that release the resources.  In that regard, it is very important for destructors to take a sane response for resources it owns. Usually that means releasing the resources directly, simply call your cleanup function during the destructor. It can also mean generating warnings, log entries, or even assertions and crashing the program if in object is destroyed without first calling the appropriate release functions.

 

This line suggests yet another interpretation: will delete the button properly on exit of the program.

Exiting a program is a special case.  When the program is exiting on a modern operating system, the OS wipes most systems (memory, disk handles, graphics resources, etc) clean automatically.  I like the quote, "When you're tearing down a building you don't need to empty the trash."  Although it is becoming less common due to certain language choices, when a program is exiting there is often no need to go through the slow and deliberate cleanup process.  For example, if you've got a bunch of audio files loaded there is usually no purpose to go through each element of the audio files and release the resources one at a time for thousands of tiny details if your program can also provide a simple "drop all the memory" command. You still need to provide the functionality for the general case of loading and unloading resource over the course of the program, this is only at application exit.

When games load millions of objects during the course of running the cleanup can take an awful long time if each one must be individually destroyed.  Some libraries provide functionality to unilaterally dump all the resources. Games can use it when dropping everything related to an unloaded level, or when ending the game.  I've seen programs (several in-house tools) that instantly terminate the program with abort() allowing the operating system to wipe the slate clean, and it exits instantly.

That is not true of all cases. If you have unsaved data exiting the program better save the user's data, or perhaps prompt the user to avoid exiting.  But if you're merely dumping data that will not be reused nor the memory reclaimed for the program's later use, cleanup isn't necessary.  Rather than deconstructing all the structures in the program one nail and one 2x4 at a time, just throw the match and let the OS sweep up the ashes.

Raw pointers should never own resources, unless the raw pointer is wrapped in a smart pointer class.  The correct way manage resources in C++ is like this:


class frame {
public:
  frame();
  
  std::unique_ptr<wxButton> button;
}

frame::frame() {
  this->button = std::make_unique<wxButton>();
  // No need to call 'delete' in the destructor.
}

However, you are using wxWidgets, where parent widgets take ownership of child widgets.  The correct code is therefore probably more like this:


class frame : public wxFrame {
public:
  frame();
  // No pointer to the button here.
};

frame::frame() {
  // Create a button...
  auto button = std::make_unique<wxButton>();
    
  // ...and relinquish ownership of it.
  this->AddChild(button.release());
}

Or, if you really need to keep a pointer to the button around:


class frame : public wxFrame {
public:
  frame();

  // Make sure the pointer is initialized to null!
  wxButton *button{nullptr};
};

frame::frame() {
  auto button = std::make_unique<wxButton>();
  this->button = button.get(); // Make a copy of the pointer.
  this->AddChild(button.release());
}

Note that none of these code examples use 'new' or 'delete' directly.

5 hours ago, a light breeze said:

 

5 hours ago, a light breeze said:

// ...and relinquish ownership of it. this->AddChild(button.release());

 

May I put this statement in the destructor?  It seems weird that ->AddChld would release the button like a delete  would and I have quite a few to do, you are sure?  I tried the search engine and didn't get any results.  This is a smart pointer?

Josheir

 

Edit: However it is looking like this solution is not a pointer at all so maybe it wouldn't need to be released?  It is using the dot operator instead of the ->.

1 hour ago, Josheir said:

May I put this statement in the destructor?  It seems weird that ->AddChld would release the button like a delete  would and I have quite a few to do, you are sure?  I tried the search engine and didn't get any results.  This is a smart pointer?

You should keep the code in the constructor.  Here is what's going on in the code:

  1. std::make_unique creates the wxButton and stores the pointer to it in a smart pointer of type std::unique_ptr called button.  At this point the button exists and is owned by the std::unique_ptr.
  2. button.release() releases the ownership of the wxButton from the std::unique_ptr.  This prevents the wxButton from being destroyed when the std::unique_ptr goes out of scope.  It is, in fact, the exact opposite of calling delete.  At this point, the button exists, but is temporarily not owned by any object.
  3. AddChild adds the wxButton as a child to another widget, in this case a wxFrame.  The parent widget assumes ownership of the button.    At this point, the button exists and is owned by its parent.
  4. When the parent widget is destroyed, its child widgets are also destroyed.  This happens automatically without user intervention.  At that point, the button will no longer exist and will not be owned by anything.

Very cool, a light breeze,

I have buttons and the they have  their first argument, which is a parent, of panel.  Panel has a first argument, which is a parent, of this.  And because the panel is in a class that inherits from wxFrame I am assuming:

Wen the object goes out of scope first panel is destroyed and than it destroys all it's child buttons.

Am I understanding right?

Josh

 

Yeah, if you pass the parent widget to the button at construction time, then you don't have to worry about deleting the button at all.  The code would look something like this:


class frame : public wxFrame {
public:
  frame();
};

frame::frame() {
  // The button will automatically be destroyed when the frame is destroyed.
  new wxButton(this, ...);
}

 

On ‎6‎/‎9‎/‎2019 at 4:22 PM, a light breeze said:

You should keep the code in the constructor.  Here is what's going on in the code:

  1. std::make_unique creates the wxButton and stores the pointer to it in a smart pointer of type std::unique_ptr called button.  At this point the button exists and is owned by the std::unique_ptr.
  2. button.release() releases the ownership of the wxButton from the std::unique_ptr.  This prevents the wxButton from being destroyed when the std::unique_ptr goes out of scope.  It is, in fact, the exact opposite of calling delete.  At this point, the button exists, but is temporarily not owned by any object.
  3. AddChild adds the wxButton as a child to another widget, in this case a wxFrame.  The parent widget assumes ownership of the button.    At this point, the button exists and is owned by its parent.
  4. When the parent widget is destroyed, its child widgets are also destroyed.  This happens automatically without user intervention.  At that point, the button will no longer exist and will not be owned by anything.

Light breeze I like how you wrote this, where do I look to get understandings like this?  Are there places?

Josheir

On ‎6‎/‎10‎/‎2019 at 3:08 AM, a light breeze said:

Yeah, if you pass the parent widget to the button at construction time, then you don't have to worry about deleting the button at all.  The code would look something like this:



class frame : public wxFrame {
public:
  frame();
};

frame::frame() {
  // The button will automatically be destroyed when the frame is destroyed.
  new wxButton(this, ...);
}

Lucky me, and the pointer can be to the left of new:  ?

wxButton * abutton = new wxButton(this, …);//this button still is automatically destroyed?

 

Josheir

And how about this:

button = new wxButton(this...);//this button Is a pointer declared in public

 

Josheir

 

The big issue with putting that in a constructor is that now it is forced upon all objects of the type, every time you create one.  You want to create a temporary then you're doing the work.  You want to create an array of 100 of them then you've got do the work 100 times when each one is constructed, and then (because you can't pass parameters to them when the array constructs them) you get to call their initialization functions again.

Repeating my advice from my first reply above, the default constructor should do no work if possible. If it must do work, it should only do enough to mark an item as empty, blank, not in use, or otherwise inactive.  

You can provide that functionality in a non-default constructor just fine, but do your future self a favor and keep the default constructors from doing work, especially from doing allocations.

This topic is closed to new replies.

Advertisement