Alright I found the problem. The base class of State, in it's destructor, it was trying to close out a font object that was also closed out in the child state class. I moved the font_close() to a new shutdown function in State, insuring that it will only be called once, I also moved the deletes in the App class to the destructor.
One more question, if I have a pointer State* _pCurState, but I never instantiate it, do I need to delete that? For example,
State* _pMenuState = new State();
State*_pCurState = _pMenuState;
delete _pMenuState:
Do I need to delete _pCurState?
Thanks for all the replies.
deleting pointers in C++
Quote:Original post by localrob2
One more question, if I have a pointer State* _pCurState, but I never instantiate it, do I need to delete that? For example,
State* _pMenuState = new State();
State*_pCurState = _pMenuState;
delete _pMenuState:
Do I need to delete _pCurState?
No. For every 'new' you should have one 'delete'. You have one new here so you can only delete once.
Also, the use of identifiers beginning with an _ is considered bad practice since they are reserved for the implementation - though yours are not likely to cause problems.
Author
Quote:Original post by Simian Man
Also, the use of identifiers beginning with an _ is considered bad practice since they are reserved for the implementation - though yours are not likely to cause problems.
I usually prefix those with m,g,etc depending on where it is. I left that off...I guess just because.
m_pCurState for a class data member, am I wrong?
Quote:Original post by localrob2Quote:Original post by Simian Man
Also, the use of identifiers beginning with an _ is considered bad practice since they are reserved for the implementation - though yours are not likely to cause problems.
I usually prefix those with m,g,etc depending on where it is. I left that off...I guess just because.
m_pCurState for a class data member, am I wrong?
Under (systems)Hungarian-notation "m_" would denote a "member variable" and "p" denotes "this is a pointer".
Some (lazy) people like to drop the "m" so it becomes "_pCurState", but as Simian Ma said, this is not standards compliant.
Other people (who like standards, but dont like hungarian) would instead use "CurState_" (the "_" on the end denotes a member variable).
There is no right way, just pick a convention you like and stick with it ;)
Quote:Original post by Simian Man
Also, the use of identifiers beginning with an _ is considered bad practice since they are reserved for the implementation - though yours are not likely to cause problems.
Although that's good advice, not all identifiers starting with an underscore are reserved. I quote:
Quote:Working Draft, Standard for Programming Language C++ (c2006/N2134)
17.4.3.1.2 Global names [global.names]
Certain sets of names and function signatures are always reserved to the implementation:
— Each name that contains a double underscore __ or begins with an underscore followed by an uppercase letter (2.11) is reserved to the implementation for any use.
— Each name that begins with an underscore is reserved to the implementation for use as a name in the global* namespace.
* Such names are also reserved in namespace ::std (17.4.3.1)
As a rule of thumb: Whenever you are about to use 'new', make sure you can explain clearly why it is needed. Many people get in a habit of dynamically allocating things all over - and then having problems sorting out the problem of when and where to deallocate them - when it is completely unnecessary.
The other features of C++ that will make your life easier are constructors and destructors. These allow you to implement the powerful idiom called RAII: Resource Acquisition Is Initialization. What it means is that you use the constructor (where the object is initialized) of an object to do your "resource acquisition" (typically, the "resource" is memory - i.e., dynamic allocation of something - but it could be a file handle - via an fstream object - or a network socket or something else). More importantly, though, is the unnamed, implicit part of the pattern - in the *de*structor of the object, you do the deallocation. The net result is that the resource's lifetime is tied to the containing object, and automatically managed that way.
Don't write Init() and Shutdown() member functions for an object. Use the constructor(s) and destructor instead. They are called automatically; that means (a) you can't forget to call them and (b) you don't have to do any extra work.
Let's suppose our Application class has a State* member, which points to its "current state". We might have derived classes of State that represent the main menu, etc. - all the states of the game. Each of these can "run" - I will spell that as "operator()" ;) - and when it's done running, will dynamically allocate the "next" state - another powerful design pattern, called "run and return successor" - an elaboration upon the State pattern that you already seem to be using.
Since everything that ever gets assigned to our 'current state' is dynamically allocated (and here, we can explicitly state the reason: the states are polymorphic, so we can't hold them by value in the Application because of "object slicing"), we have to delete for every new. The way we'll do this is:
- In the constructor, we new the initial state.
- Every time we invoke the current state, we new the next state (indirectly), and delete the current state (within the Application code).
- In the destructor, we delete the final state.
It is fairly easy to look over this structure and verify that every newed thing gets deleted exactly once. So, we proceed, with something like:
The other features of C++ that will make your life easier are constructors and destructors. These allow you to implement the powerful idiom called RAII: Resource Acquisition Is Initialization. What it means is that you use the constructor (where the object is initialized) of an object to do your "resource acquisition" (typically, the "resource" is memory - i.e., dynamic allocation of something - but it could be a file handle - via an fstream object - or a network socket or something else). More importantly, though, is the unnamed, implicit part of the pattern - in the *de*structor of the object, you do the deallocation. The net result is that the resource's lifetime is tied to the containing object, and automatically managed that way.
Don't write Init() and Shutdown() member functions for an object. Use the constructor(s) and destructor instead. They are called automatically; that means (a) you can't forget to call them and (b) you don't have to do any extra work.
Let's suppose our Application class has a State* member, which points to its "current state". We might have derived classes of State that represent the main menu, etc. - all the states of the game. Each of these can "run" - I will spell that as "operator()" ;) - and when it's done running, will dynamically allocate the "next" state - another powerful design pattern, called "run and return successor" - an elaboration upon the State pattern that you already seem to be using.
Since everything that ever gets assigned to our 'current state' is dynamically allocated (and here, we can explicitly state the reason: the states are polymorphic, so we can't hold them by value in the Application because of "object slicing"), we have to delete for every new. The way we'll do this is:
- In the constructor, we new the initial state.
- Every time we invoke the current state, we new the next state (indirectly), and delete the current state (within the Application code).
- In the destructor, we delete the final state.
It is fairly easy to look over this structure and verify that every newed thing gets deleted exactly once. So, we proceed, with something like:
// And please do note several other minor things that I do differently - naming,// scoping of the Application instance etc.class Application { State* current; // etc. // An Application instance should never be copied or assigned, so we // can do the following to prevent that: Application(const Application&); // DO NOT IMPLEMENT THESE. Application& operator=(const Application&); public: Application() : current(new StartMenu()) {} // etc. ~Application() { delete current; // and anything else that needs to happen at "shutdown". } // Invoke the current state, and communicate whether the game is still running bool operator()() { // This is "dereference the pointer to get the state, and call its // operator(), passing it a reference to myself as an argument." State* next = (*current)(*this); delete current; current = next; return !current.isFinal(); // Another way is to have the State operator() return NULL if the game // should end. We check for null-ness here, and then there is no longer // a need for a destructor, since the NULL doesn't need deleting. }};int main() { // We don't need to dynamically allocate the *application* instance. Application game; // "init" work automatically done. while (game()); // "stuff happens each frame"; the loop is empty, as // all the work is done via the Application::operator(). // "shutdown" work automatically done at the end of the function.}This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
