cleaning up a state

Started by
12 comments, last by GrimV5 10 years, 2 months ago

button1 and button2 are both RectangleShape objects from the SFML library. I tried putting the following lines in TidyUp():
delete button1;
delete button2;


button1 and button2 are RectangleShape objects from the SFML library, but they weren't allocated on the heap with new. Therefore, no delete is necessary:


int main() {
    sf::RectangleShape a = sf::RectangleShape(sf::Vector2f(640.0f,480.0f));
    sf::RectangleShape* b = new sf::RectangleShape(sf::Vector2f(640.0f,480.0f));

    // Do stuff with a, b

    delete b;
} // a is destroyed automatically

Advertisement
As fastcall22 mentioned, only delete what you new - and if you're a beginner, don't new or delete at all. And if you're a professional, and using modern C++ techniques, you'd use smart pointers 95% of the time anyway so you'd still mostly ignore new and delete.

C++ doesn't have a garbage collector, what are you talking about?

This only proves to me that my C++ teacher wasn't that great, he never clarified certain things so I was left to assume.

It's the student's job to ask questions if they don't understand.
It's the teacher's job to answer questions - that's what they are there for (otherwise, you might as well just read a book).
The teacher can't answer questions that the students don't ask. wink.png

Plus, C++ does automatically collect some memory. It doesn't have a "garbage collector", but it still frees memory automatically in certain circumstances. Maybe your teacher was trying to explain how C++ memory was collected, and having heard the phrase "garbage collector" sometime in the past, you might've automatically assumed that the teacher was talking about garbage collection?

I'm guessing this is the case, because this is exactly the problem you are having with your code: You think certain memory isn't getting freed that C++ is already freeing for you, and you're trying the manually free memory that has already been freed (or that will automatically be freed later). smile.png

As popular as it is to do so, don't assume your teacher is ignorant. Just ask questions if you don't understand. If, after asking questions, something still doesn't make sense, then ask on these forums and we can clarify.

Here's how basic C++ variables work: (Don't roll your eyes, this is important to understand!)
int x = 0;
That line of code creates an integer named 'x', that is initialized to '0'. It will automatically be freed when the execution of the code reaches the end of its scope.

Example:
void func()
{
     int a = 20;
 
     if(true)
     {
          int b = 70;
          
     } //The variable 'b' gets freed after this point.
    
     int c = 0;
     
} //The variables 'a' and 'c' get freed after this point.

If you do this:
void func()
{
     int a = 20;
 
     delete &a; //Programmer  manually frees 'a' (not good).
     
} //Compiler automatically frees 'a'. But since was *already* freed, the program will either
  //crash (if you're lucky) or do unexpected crazy stuff that's really hard to track down and debug.
For every variable in your code, either you're managing its memory, or the compiler is. But not both. Every variable should have an owner: Who owns it? The function? The class? Or are you manually managing it?

Now let's look at the lifetime of variables in structs: (classes behave the same way)
struct MyStruct
{
     int x;
};
'x' is owned by 'MyStruct'. When 'MyStruct' has its memory freed, 'x' will also be freed.

Observe:
void func()
{
     MyStruct myStruct;
     myStruct.x = 20;
     
} //'myStruct' is freed here. Because 'myStruct' is freed, 'myStruct.x' is also freed at the same time.

Now classes also have constructors and destructors, which is important to use. All your 'Load()' code should go in the class's constructor. That's what it's for.
All your 'TidyUp()' code should go in the class's destructor. That's why destructors exist.
//This is a constructor. It's a function called when a class gets created.
MainMenuState::MainMenuState()
{
    
}
 
//This is a destructor. It's a function called when a class gets destroyed.
MainMenuState::~MainMenuState()
{
    
}
Unless you're manually doing something that needs to be manually undone, you don't really need to use a destructor in this MainMenuState class.

It's the student's job to ask questions if they don't understand.
It's the teacher's job to answer questions - that's what they are there for (otherwise, you might as well just read a book).
The teacher can't answer questions that the students don't ask. wink.png

This is not the issue and I don't assume he's ignorant. It's just that english is not his first language and while he can speak it pretty well, he still has an issue understanding what other people ask or say to him. I've physically walked up to the board to write (even draw) out my problem and barely half the time does he understand. And while his TA CAN speak english, he is near impossible to get a hold of. So I resort to an C Reference handbook and C++ forums to get my answers.

As fastcall22 mentioned, only delete what you new - and if you're a beginner, don't new or delete at all. And if you're a professional, and using modern C++ techniques, you'd use smart pointers 95% of the time anyway so you'd still mostly ignore new and delete.

C++ doesn't have a garbage collector, what are you talking about?

This only proves to me that my C++ teacher wasn't that great, he never clarified certain things so I was left to assume.

It's the student's job to ask questions if they don't understand.
It's the teacher's job to answer questions - that's what they are there for (otherwise, you might as well just read a book).
The teacher can't answer questions that the students don't ask. wink.png

Plus, C++ does automatically collect some memory. It doesn't have a "garbage collector", but it still frees memory automatically in certain circumstances. Maybe your teacher was trying to explain how C++ memory was collected, and having heard the phrase "garbage collector" sometime in the past, you might've automatically assumed that the teacher was talking about garbage collection?

I'm guessing this is the case, because this is exactly the problem you are having with your code: You think certain memory isn't getting freed that C++ is already freeing for you, and you're trying the manually free memory that has already been freed (or that will automatically be freed later). smile.png

As popular as it is to do so, don't assume your teacher is ignorant. Just ask questions if you don't understand. If, after asking questions, something still doesn't make sense, then ask on these forums and we can clarify.

Here's how basic C++ variables work: (Don't roll your eyes, this is important to understand!)

int x = 0;
That line of code creates an integer named 'x', that is initialized to '0'. It will automatically be freed when the execution of the code reaches the end of its scope.

Example:

void func()
{
     int a = 20;
 
     if(true)
     {
          int b = 70;
          
     } //The variable 'b' gets freed after this point.
    
     int c = 0;
     
} //The variables 'a' and 'c' get freed after this point.

If you do this:

void func()
{
     int a = 20;
 
     delete &a; //Programmer  manually frees 'a' (not good).
     
} //Compiler automatically frees 'a'. But since was *already* freed, the program will either
  //crash (if you're lucky) or do unexpected crazy stuff that's really hard to track down and debug.
For every variable in your code, either you're managing its memory, or the compiler is. But not both. Every variable should have an owner: Who owns it? The function? The class? Or are you manually managing it?

Now let's look at the lifetime of variables in structs: (classes behave the same way)

struct MyStruct
{
     int x;
};
'x' is owned by 'MyStruct'. When 'MyStruct' has its memory freed, 'x' will also be freed.

Observe:

void func()
{
     MyStruct myStruct;
     myStruct.x = 20;
     
} //'myStruct' is freed here. Because 'myStruct' is freed, 'myStruct.x' is also freed at the same time.

Now classes also have constructors and destructors, which is important to use. All your 'Load()' code should go in the class's constructor. That's what it's for.
All your 'TidyUp()' code should go in the class's destructor. That's why destructors exist.

//This is a constructor. It's a function called when a class gets created.
MainMenuState::MainMenuState()
{
    
}
 
//This is a destructor. It's a function called when a class gets destroyed.
MainMenuState::~MainMenuState()
{
    
}
Unless you're manually doing something that needs to be manually undone, you don't really need to use a destructor in this MainMenuState class.

Well, I don't actually use a constructor for my states anyway, here is my initialstate header:


#ifndef InitializeState_h
#define InitializeState_h
#include "State.h"
#include "SFML/Graphics.hpp"


class InitializeState : public State{
public:

void Load();
void Handling(SEngine* gameEng);
void Paint(SEngine* gameEng);
void Update(SEngine* gameEng);
void TidyUp();
void Halt();
void Continue();
static InitializeState* Initialize(){
return &gameStart;
}


protected:
InitializeState() {}

private:
static InitializeState gameStart;
int ready;
sf::RectangleShape shape;
};


#endif
Here is my main that calls the Initialize function:

#include "SEngine.h"
#include "InitializeState.h"


using namespace std;

int main(){
  char word[] = {'T','e','s','t','i','n','g'}; //ignore, just for testing purposes
  SEngine engine;
  engine.Load(word);
  engine.changeState(InitializeState::Initialize());

  while(engine.isRunning()){
   while(engine.getWindow()->isOpen()){
    engine.Handling();
    engine.Update();
    engine.Paint();
   }
  }

  return 0;
};

And here is my state engine class:


#include "SEngine.h"
#include <SFML/Graphics.hpp>
#include "State.h"


void SEngine::Load(const char* title, int xsize, int ysize){
  running = true;
  width = xsize;
  height = ysize;
  window.create(sf::VideoMode(width,height), "Game Engine");
};

void SEngine::changeState(State* state){
  if(!states.empty()){
   states.back()->TidyUp();
   states.pop_back();
  }

  states.push_back(state);
  states.back()->Load();
};

void SEngine::Handling(){
  states.back()->Handling(this);
};

void SEngine::Update(){
states.back()->Update(this);
};

void SEngine::Paint(){
  states.back()->Paint(this);
};

bool SEngine::isRunning(){
  return running;
};

void SEngine::endGame(){
  running = false;
};

void SEngine::PushIt(State* gstate){
  if(!states.empty()){
   states.back()->Halt();
  }

  states.push_back(gstate);
  states.back()->Load();
};

void SEngine::PopIt(){
  if(!states.empty()){
   states.back()->TidyUp();
   states.pop_back();
  }

  if(!states.empty()){
   states.back()->Continue();
  }
};

void SEngine::TidyUp(){

  while(!states.empty()){
   states.back()->TidyUp();
   states.pop_back();
}


};

This topic is closed to new replies.

Advertisement