30 replies to this topic

[Ubermeowmix]

If I create a pointer reference in a class:

private:
string* m_pName;
int* m_pHealth;

Animal::Animal(const string& name = "", const int& health = 2)
{
m_pName = new string(name);
m_pHealth = new int(health);
};

Animal::~Animal()
{
delete m_pName;
m_pName = 0;
delete m_pHealth;
m_pHealth = 0;
};

then call this from an external source:

Animal MyAnimal1("Denzel", 12);

Does the object still exist on the HEAP, or do I still need to delete it? This is the bit that confuses me, as if it still exists in memory then surely I still need to get rid of it... don't I?

Any help would be appreciated.

[dmatter]

You simply can't do that. At the call site you created a couple of temporaries, they're only good for the scope of that function (which in this case is the constructor). By stashing those pointers into the class variables you have not extended the lifetime of the temporaries, all that happens is that when the constructor returns your two member pointers are left pointing to un-allocated memory.

Edit: Whoops I didn't spot you were creating copies on the heap. Silly me.

Edited by dmatter, 25 July 2012 - 04:42 AM.

[Ubermeowmix]

So that's a memory leak?!

So do I return a pointer reference to the call of Animal:
*pMyAnimal1 = Animal tempAnimal("Denzel", 12);

then only when I:
delete pMyAnimal1;
and kill dangly pointers:
pMyAnimal1 = 0;

does the memory cease to exist, leaving no floaty memory in the system anymore!

[larspensjo]

There are three different things allocated in your example. A string, an int, and an object (MyAnimal1) of class Animal. The allocation of MyAnimal1, which consists of two pointers, depends on where the declaration is. If the declaration 'Animal MyAnimal1("Denzel", 12);' is in a function, then the object will be allocated on the stack. That also means that it will be deallocated when the function body goes out of scope. That is also when the destructor will be called.

However, if MyAnimal1 is declared as a global parameter, it will be located in the BSS area (pre allocated by the linker). It will not get deallocated until you program terminates.

For the two pointers, the situation is different. The content they point top will always be on the heap. And they will be freed when the destructor is called.

Why are you using a pointer to a string? Why not use the string itself in the class?

You don't need to assign 0 to the pointers in the destructor. The memory will be freed anyway. However, it may be a good practice, helping you to catch errors. Normally, you must not refer to an object that has been destructed, but it is possible if you have a pointer to it. The source code you show above has no such pointer, though.

[Mussi]

You do not have to delete MyAnimal1, it'll be allocated on the stack. The memory for the member variables are allocated on the heap, but the destructor takes care of those. In short, if you don't 'new' a variable you don't have to delete it(generally).

I don't see any reason to use a pointer to a string or integer in this scenario though, I'd change the member variables to non-pointers.

[BeerNutts]

You simply can't do that. At the call site you created a couple of temporaries, they're only good for the scope of that function (which in this case is the constructor). By stashing those pointers into the class variables you have not extended the lifetime of the temporaries, all that happens is that when the constructor returns your two member pointers are left pointing to un-allocated memory.

This isn't right. The only thing that would be "temporary" is the object MyAnimal1, assuming it's declared in a function. In that case, MyAnimal1 will be created on the stack, but the member variables would be allocated from the heap. When your function ends, the destructor is called, and the member variables are removed from the heap. You don't need to NULL them out.

Please don't up vote something if you don't understand what's being said.

OP, I hope that answers your question.

[Ubermeowmix]

Thanks for all the help guys, i guess i've learned a lesson on taking advice as well as pointers

Why are you using a pointer to a string? Why not use the string itself in the class?

Just trying to get my head around what I can and can't do really, i want to be creating entities with alterable stats later. So I'm trying to understand the correct ways to load and unload them when I get to generating 100+ of them.

One more question if I can, when I do load multiple objects, whats the best method to use. Is it a vector of class objects? Or is there another standard method I'm not aware of?

[FredOrJoeBloggs]

Does the object still exist on the HEAP, or do I still need to delete it? This is the bit that confuses me, as if it still exists in memory then surely I still need to get rid of it... don't I?

MyAnimal1 is created on the stack
The pointers within the class are created on the stack.
The memory that the pointers point to is created on the heap.

Because of the way you have written you destructor all the memory will be released when it is called, so you have done everything right. Especially the redundant nulling of the pointers, its good practice, but might have performance issues in games.

However, I can't see why you are using dynamic memory for this data, why not just let the class members be a string and an int? I apreciate you may have simplified it for the question, in which case ignore this comment.

[larspensjo]

One more question if I can, when I do load multiple objects, whats the best method to use. Is it a vector of class objects? Or is there another standard method I'm not aware of?

It depends a little on what you want to do. There are a couple of possibilities:

Animal myAnimals[10];

This will allocate space for 10 objects. Again, on the stack if locally in side a function. All 10 objects will have the constructor executed, but with no arguments. Of course, it is better to replace '10' with MAX or something similar.

Animal *myAnimals[10];

This will allocated 10 pointers, initialized to random values. You need to allocate each Animal of your own, and assign it. For example,
myAnimals[0] = new Animal("Denzel", 12);
You will now have to make sure you deallocate each pointer eventually.

Animal *myAnimals = new Animal[10];

This will allocate one space for 10 objects. The advantage compared to the first example is that it will not be deallocated as soon as the declaration goes out of scope. The disadvantage is that you have to deallocate them yourself eventually, doing "delete[] myAnimals;".

std::vector<Animal> myAnimals;

This is maybe the best solution. It will define a vector of Animals, that can be expanded dynamically. It keeps track of the number of elements, which means you don't need a separate counter. It will be deallocated automatically when you go out of scope, so you will not need to do any delete on your own. The variable myAnimals is a "first class object" (compared to arrays in C++ that are handled as pointers). That means you can easily make a copy of it. There is some overhead, but it is very low, so don't worry about performance until you have measurments showing otherwise.

std::unique_ptr<Animal> myAnimal;

This is a smart pointer to one object. So not really about arrays, but nice to know anyway. You can use it with the same syntax as if it was a normal pointer. It has a few interesting attributes: It will be deallocated automatically and it can't be copied elsewhere. It may look like a disadvantage, but that way you can guarantee that there is no danging pointer stashed away somewhere else when the object is deleted.

[dmatter]

You simply can't do that. At the call site you created a couple of temporaries, they're only good for the scope of that function (which in this case is the constructor). By stashing those pointers into the class variables you have not extended the lifetime of the temporaries, all that happens is that when the constructor returns your two member pointers are left pointing to un-allocated memory.

This isn't right.

Thanks for spotting my mistake BeerNutts! I hadn't noticed that the constructor is actually constructing copies; I misread it as a straight pointer assignment. My bad.

[Ubermeowmix]

That's awesome guy's I really appreciate the time you've taken to explain that.

Particularly like your input larspensjo, it's a great explanation.

Right I'm off to make more spelling mistakes and get tied up in knots lol.

[Ubermeowmix]

One last thing.

Because of the way you have written you destructor all the memory will be released when it is called, so you have done everything right. Especially the redundant nulling of the pointers, its good practice, but might have performance issues in games.

Oh in what sense is that a bad thing, as this is definitely games coding!

However, I can't see why you are using dynamic memory for this data, why not just let the class members be a string and an int? I apreciate you may have simplified it for the question, in which case ignore this comment.

So if i want to store floating point numbers and strings they can just be basic type def calls and that will save on CPU power will it?, what would I need to use the dynamic memory for? just the initial instantiation of the class (or Animal) into a vector (as in larspensjo's example), and then delete it if the object dies in game for instance.

[fastcall22]

what would I need to use the dynamic memory for?

To control the lifetime of objects.

When an object is created on the stack, its lifetime is bound to the scope it resides in. Similarly, the lifetime of the members inside an object is bound to the lifetime of the object. Because the lifetime of objects on the stack are predictable (LIFO ordering), allocating objects on the stack is fast. However, objects aren't always allocated and deallocated in order. The heap serves to manage dynamically allocated objects that can be allocated an deallocated in any order. By using dynamically allocated memory, you can control when and where to end an object's life.

[Ubermeowmix]

Thanks fastcall, upon testing the above I've hit on another 'wall'!

Using a vector creates a dynamic list of objects, and I want to kill off these objects by reducing their lifeforce muhahahaha *ahem*.
When I iterate through this list and remove the dead ones by calling the destructor to kill them off, the vector fails as the original scope of the for loop fails. How do I say check them all then delete the ones that are dead for a new list?

[fastcall22]

The destructor is called automatically; do not manually call the destructor*.

Removing elements from a vector while iterating looks something like this:

vector<Object*> objects;
for ( vector<Object*>::iterator itr = objects.begin(); itr != objects.end(); /* empty */ ) {
    if ( itr->shouldBeRemoved() ) {
        delete *itr; // Release memory allocated for the object pointed by the pointer in the iterator
        itr = object.erase( itr );
    } else
        itr ++;
}

* Unless the object was constructed using placement-new.

Edited by fastcall22, 25 July 2012 - 02:56 PM.

[Ubermeowmix]

Ah that Might be why it's dying lol!. and I was using a standard for loop on the size of the vector which may explain the other error. Thanks.

[larspensjo]

Notice the difference between "vector<Object*> objects;" and "vector<Object> objects;" (the '*').

The first is a vector of pointers to objects (where you have to do new/delete on individual objects), and the second is a vector of objects (no new/delete needed, it is all automatic). As mentioned above by fastcall22, what you use depends what requirements you have on the lifetime of the individual objects.

Notice also that std::vector is efficient when adding elements, but less efficient when removing elements that are not in the end. It is done by moving all subsequent elements "down", to overwrite the space of the removed element.

If you want to have long lists where you add and remove elements randomly, using std::list may be better. In practice, it is implemented as a linked list. While insertion and removals are quick, it is not possible to do indexing. There are a couple of other nice containers, each optimized for a special purpose.

As the API to the container types are similar, but not identical, it means it is usually easy to start the implementation using one container type, and change it later when you find you have special performance requirements. For example, iterating over a vector as fastcall22 shows is the same syntax that is used to iterate over a std::list. And then, in C++11, there is special syntactical support for iteration statements.

[Ubermeowmix]

vector objects;
for ( vector::iterator itr = objects.begin(); itr != objects.end(); /* empty */ ) {
if ( itr->shouldBeRemoved() ) {
delete *itr; // Release memory allocated for the object pointed by the pointer in the iterator
itr = object.erase( itr );
} else
itr ++;
}

This is evaluating them all to death when only 1 of them is at zero (m_Health <= 0), I don't get why?

[Ubermeowmix]

If you want to have long lists where you add and remove elements randomly, using std::list may be better. In practice, it is implemented as a linked list. While insertion and removals are quick, it is not possible to do indexing. There are a couple of other nice containers, each optimized for a special purpose.

This is the bit i'm trying to figure out, I wan't lots of baddies being generated and killed so i'm looking for the most efficient way to do it. If it's lists then I'll go off and research them over vectors.

What I essentially want are lots of objects all with specific data that can be called on depending on the users distance from that object. Like an interaction thing, so the calls for checking stats etc will be less the further away the user is from them.

[Mussi]

This is the bit i'm trying to figure out, I wan't lots of baddies being generated and killed so i'm looking for the most efficient way to do it. If it's lists then I'll go off and research them over vectors.

I'm going to put words in your mouth and say that you don't want to do that, you just want to get it working. Figuring out the most efficient way to do it is going to require you to write both methods and profile them. You only want to do that if there's an actual need for it(i.e. your game is running to slow). Stick with vectors for now.

This is evaluating them all to death when only 1 of them is at zero (m_Health <= 0), I don't get why?

Could you paste the relevant code?