# Problem with Virtual Function

## Recommended Posts

signal_    366
I recently have begun to use virtual functions. I am having a problem at run time. Can someone please check my syntax and methods? See below for an explanation of the outcome. Here is my code. I have broken it up into files because that is how it is in my work directory. I think the problem may lie with my syntax. entity.h file:
class Entity
{
protected:
int xCo; int yCo;   // coordinates for the entity

public:

Entity( eID );

~Entity();

virtual void update();
};


Entity.cpp file:
#include "entity.h"

Entity::Entity( eID )
{
// do stuff
}

Entity::~Entity()
{
}

void Entity::update()  // Update Function -- called once per frame.
{
std::cout << "***Update" << std::endl;
}


Then I have a derived class that I want to use. derive.h file:
#include "entity.h"

class Derive: public Entity
{
public:
Derive( eID );
~Derive();

void update();

};


derive.cpp file:
#include "derive.h"

Derive::Derive( eID ):Entity( eID )
{
//

}

void Derive::update()
{
std::cout << "UpdateDerive!!!!!!!!!!!!!!!" << std::endl;
}

Derive::~Derive()
{
}


Basically, I create a vector: std::vector<Entity*> entities; Then I call a function: void EntityList::updateEntity() { for( int i = 0; i < entities.size(); i++ ) // i < entity_ID { entities[i]->update(); } } When I run the program it says MS Windows detected a problem & closes. Compiles and links without problem. I also completely rebuilt the project. When I remove the virtual keyword, i.e. type "void update();" instead of "virtual void update();" in the entity.h file the program runs and exits fine. Of course the added functionality of derived classes isn't there but it runs. What am I doing incorrectly? Please assist.

##### Share on other sites
Zahlman    1682
You must make the destructor of the base class 'virtual'. (I assume you do actually have code somewhere that allocates entities with 'new' and puts them into the vector, and which calls 'delete' on those pointers later.) (You must also ensure that each created object is only deallocated once.)

Basically: the destructor is a member function, too. If it's not 'virtual', then it won't do any dynamic dispatch, which means the base destructor gets called on the pointed-at object, even if it's actually an instance of the derived class. And that's bad.

##### Share on other sites
SiCrane    11839
How are you adding entities to your vector? It sounds like you've got some bum pointers in there.

##### Share on other sites
Cornstalks    7030
As a rule, a class with a virtual function should have a virtual destructor. Here is a link that more correctly explains the rule (section 20.7). Other than that, I don't know what the problem could be. I assume that entities is filled with valid objects, of course.

Ah fetch! Ninja'd twice!

##### Share on other sites
MaulingMonkey    1728
1) Run the program in your debugger to, among other things, find out exactly where the crash occurs (clicky)
2) Entity must have a virtual destructor. As is, the following code may explode:
Entity* foo = new Derive();delete foo;

3) Please always copy & paste real source code. (If you really are directly copy and pasting a class named "Derive" from what you're compiling, you may want to start naming your classes better ;-))

If the crash is actually occurring on the update() call, I'm guessing your vector contains uninitialized, deleted, or otherwise invalid pointers. Since update() doesn't access any of the member variables of the class (or functions that do so themselves), the implicit "this" pointer isn't actually being used when update isn't virtual. By making the function virtual, your compiler beings using the implicit "this" pointer to get the virtual table so it can call the correct function for the class you're using.

A way to confirm this would be to make Entity::update print out xCo and yCo instead of just "***Update". This would cause the code to crash even if the function isn't virtual if my guess is right.

Edit: Ninja'd thrice <_<

##### Share on other sites
signal_    366
Wow! Thanks for all the quick replies. I was asked about how I am adding entities to my vector, so I will include that now:

void EntityList::addEntity(int x, int y){    Entity* temp_entity = new Entity( entity_ID );    // now increment ID entity!    entity_ID++;    temp_entity->SetX( x );    temp_entity->SetY( y );    entities.push_back( temp_entity );   // create the entity!    delete temp_entity;}

I am afraid that I need to read through all the responses thus far to offer a better reply. But until then thanks for all the responses!

##### Share on other sites
SiCrane    11839
Why on earth are you deleting it after you add it?

##### Share on other sites
signal_    366
SiCrane,

I am obviously confused here... By deleting it I am just ruining what I just did, right?

What I want is a container for the entities and all these different entities to use their respective update functions.

So, do I need to store all my entities in a vector and store my entity pointers in another? Sorry I am confused.

##### Share on other sites
Cornstalks    7030
Quote:
 Original post by signal_SiCrane,I am obviously confused here... By deleting it I am just ruining what I just did, right?What I want is a container for the entities and all these different entities to use their respective update functions.So, do I need to store all my entities in a vector and store my entity pointers in another? Sorry I am confused.

No, the problem is you create the object, store it, and then delete it. That means that the vector now contains a bunch of deleted objects. Trying to use them crashes your program. You should do something more like

void EntityList::addEntity(int x, int y){    Entity* temp_entity = new Entity( entity_ID );    // now increment ID entity!    entity_ID++;    temp_entity->SetX( x );    temp_entity->SetY( y );    entities.push_back( temp_entity );   // create the entity!}

And in the destructor of the class (or somewhere that it will be called so your memory doesn't leak), iterate through the vector and delete the objects, like this:

for (int i = 0; i < entities.size(); i++){    delete entities[i];}entities.clear();

##### Share on other sites
signal_    366
Mike, by removing that delete line and making the changes you outlined everything works now the way I intended.

I am weak with pointers and usually avoid using them, which explains my confusion with new and delete. Also there are some aspects of OOP that I am very shaky with. I come from a C background and was not a comp sci student at uni (this is obvious to all of you, but I am being complete...lol).

I have a lot to learn and I truly appreciate everyone who took the time to respond to my post. Thank you so much!

Unfortunately, I must run out for a quick errand so I will write more when I return.

##### Share on other sites
MaulingMonkey    1728
This fundamentally has little to do with OOP. new and delete are roughly equivalent to C's malloc and free, if you're used to using those. (You can't free() what you new or delete what you malloc, and the C++ versions also call constructors and destructors, but both sets of operations (de)allocate memory.)

It's worth noting that just removing the delete will still leave you with a memory leak unless you delete the objects somewhere later in your program (when you're all done using those pointers). Some ways to automate the deletion of these objects when you're done with them:

1) Use std::vector< *some smart pointer type here* > instead of std::vector< T* > ('dumb pointers'). Basically, your current vector doesn't think it "owns" the pointed to objects, so it doesn't delete them when they're removed or the vector is destroyed. However, it does own the pointers themselves. Example:

{    int *foo = new int(42);} // foo is leaked{    std::vector< int* > foo;    foo.push_back( new int(42) );} // foo[0] is leaked{    int *foo = new int(42);    delete foo; // if we remember this, we don't leak}{    std::vector< int* > foo;    foo.push_back( new int(42) );    delete foo[0]; // this will quickly get tedious}{    boost::shared_ptr<int> foo( new int(42) );    // when the boost::shared_ptr is destroyed by the local scope,    // it automatically calls delete on the raw pointer inside it's destructor.    // Since it's automatic, you can't forget to delete it!} // no leak here{    // works with std::vector fine:    std::vector< boost::shared_ptr<int> > foo;    foo.push_back( boost::shared_ptr<int>( new int(42) ) );} // no leak here either, since std::vector destroys the boost::shared_ptr which deletes the raw pointer which destroys the int.

Eventually, shared_ptr is actually going to become part of the Standard C++ Library, but in the meantime, you can visit The Boost Website.

2) Use boost::ptr_vector, which does think it owns the objects pointed to by pointers. I'll leave reading up on the documentation to you -- it's probably a little more appropriate, but boost::shared_ptr is very useful outside of containers as well, so I figured I'd expend my efforts explaining the more generic tool.

Both options end up being pretty similar anyways.

##### Share on other sites
Zahlman    1682
Quote:
 Original post by signal_SiCrane,I am obviously confused here... By deleting it I am just ruining what I just did, right?What I want is a container for the entities and all these different entities to use their respective update functions.So, do I need to store all my entities in a vector and store my entity pointers in another? Sorry I am confused.

You can't just "store all [the] entities in a vector" because they're of varying types (base and derived). That's why you made a vector of pointers in the first place (right?).

When you write 'new Whatever()', you are saying "Please allocate a Whatever-sized chunk of memory somewhere, and set it up to contain a Whatever, and give me back a pointer to it." That suffices for "storing" your entities; they are in memory, and you know where they are. So then you go ahead and put those pointers in a vector, so that you can remember all the pointer values.

When you write 'delete some_variable', you are saying "I am done with the thing pointed at by some_variable. Please clean it up (run the destructor) and then you can have that memory back to use for something else" (where "something else" could be another 'new' call by your program, a request from another program or the OS itself, etc.) The variable now has the same value, which is no longer a valid pointer, because the pointed-at thing no longer exists. You can no longer use that pointer for anything meaningful (nor any other pointer which happened to be pointing to the same object).

A pointer is only a pointer. It, itself, knows nothing about the thing that it points at, except that, in the source code, it has a type (so it knows what kind of thing it's pointing at, if it's validly pointing at something).

So you don't want to 'delete' them until after you're actually done with the pointed-at instances. In your case, that will be at the point where you clean up the vector, i.e. the EntityList destructor.

##### Share on other sites
signal_    366
Thank you all very much for yr replies. Everything is working now. This code forms the basis of a game entity system and I am almost to the point where I can integrate this system into my existing game code.

I am still going through all of the responses. Anyway, I apologize for my silly mistake, but please know that I now have a basic understanding of what's going on. As long as I do not quit I will always make progress.

And, MaulingMonkey, regarding yr comment about posting my source: I definitely "doctored" my code for posting on here. It is about 500 lines and I did not want to torture any potential responders. But, in the future, I will try to post code in a more verbatim style so as to efficiently assist in the diagnosis of my desultory code.

As far as the boost stuff goes, the smart pointers and whatnot: I am down with this. Since I now have my basic system down I can play around with these more elegant solutions, i.e. boost's smart pointers.

Best wishes to all.