irrlicht in c++ class code and destructor

[paooolino]

Hi guys. just a nooby question

Let's say I make a c++ program in wich I define my own Game class. Obviously, Game class has members like
- IrrlichtDevice* device;
- IVideoDriver* driver;
..and so on.

The device is created in the Game class constructor. A run() method contains the game loop.

The main program is something like this:

Code: Select all

int main(){
      Game g;
      g.run();

      return(0);
}

so the question is.. How do I behave with the Game class destructor?

My answer: I just put device->drop() in the destructor, so I don't need to deallocate pointers like driver and device - because they're been destroyed by device->drop().
If needed, I just have to delete my custom pointers that are not part of Irrlicht Engine.

A mate suggested me a piece of code like this after dropping the device:

Code: Select all

if ( (device) != NULL ) delete (device);

but I thin it's not needed.
What do you think guys?

[CuteAlien]

Calling delete after you called drop would result in a runtime error. The call do drop() does already delete the object.
So in short - you are doing it the right way :-)

[rogerborg]

A mate suggested me a piece of code like this after dropping the device

Mates don't tell mates to delete dropped references.

[vitek]

One slightly unrelated thing to note. You do not need to check a pointer for NULL before calling delete on it. It is perfectly legal to call delete on a NULL pointer.

[Johan]

i see nothing wrong with the way your mate suggested because firstly if the device was dropped it would skip your if statement ,no harm done .the delete code would only come into effect when device does not equal null and only after device-drop returns back to your class code ..

[JP]

Johan... device->drop() does not make device == NULL... that seems to be what you're saying and you MUST realise that's wrong right now or else the world will implode and i really don't want to die a horrible death right now

[Johan]

jp. no what im saying is when device drop returns and now equals null ,then the if statement would not do any harm what so ever since device already deleted so nothing wrong in putting it in there if he likes to .the if statement would be equal to -device is null- . i cant see why there would be an exception error

[Johan]

sorry i ment runtime error and not exception... and jp i think you misunderstood my post on the last part ,what was said is that -only- when the statement equals true ,hence the device not being null would the delete part of code execute .but im pretty sure you and i and others know that would not happen ,so yeah that was suppose to be a short understandable post without going in it to deeply but hopefully im much clearer on it now

[Seven]

my own idiosyncracies......

Code: Select all

#define CS_INIT(x) x=0;
#define CS_SAFE_DELETE(x) if(x) delete(x); x=0;
#define CS_SAFE_DROP(x) if (x) x->drop(); x = 0;
#define CS_SAFE_RELEASE(x) if (x) x->release(); x = 0;
#define CS_SAFE_REMOVE(x) if (x) x->remove(); x = 0;

but then again, I seriously love defines

I even do this....

Code: Select all

#define ADD_PUBLIC_SETGET(type,name,nametag)	public  : type name; virtual type Get##nametag() { return name; }; virtual void Set##nametag(type d) { name = d; };

which is used like this

ADD_PUBLIC_SETGET(int,m_Health,Health);

and gives me this

Code: Select all

int m_Health;
virtual int GetHealth() { return m_Health; };
virtual void SetHealth(int d) { m_Health = d; };

[CuteAlien]

what im saying is when device drop returns and now equals null

You confuse objects and variables which contain pointers to objects. The object is droped (or deleted). That means on delete it's destructor is called and the memorymanager of your OS will be told that it can now re-use the memory which was blocked by this object so far. Nothing is set to 0. Especially not the pointer variable. It will still contain the same (non-null) value to the address where that object was allocated. So unless it's explicitely set to 0 - which is certainly in basically every circumstance a very good idea - it won't be 0 and calling another delete on it will therefore be an error.

[JP]

Yup, cutealien explained what i was saying more.

When you drop something it means you're done with it and you don't care about it anymore so you should never use that pointer again (unless it's reinitialised to something else of course). So you should always set it to to NULL after calling drop, same goes for when calling remove on a node or calling delete on a non-irrlicht pointer.

Much like Seven showed with his SAFE #defines

[Johan]

jp .thanx for the way you corrected .i just hate when people put down the newcomers , i just might stick around on irrlicht forum just because you showed theres hope for it by being so nice as to help instead of crush a newcomer by sarcasm etc.-had plans to switch to ogre due to ugly remarks forwarded time after time to newcomers in the irrlicht comunity- p.s im not realy standing by my silly programing point.

[rogerborg]

jp .thanx for the way you corrected .i just hate when people put down the newcomers

Well played, JP, well played. I take one little break from posting, and you take advantage of it to encourage and engage with people? Now I know where we stand.

Just so we're super clear, most Irrlicht objects are derived from IReferenceCounted. IReferenceCounted::drop() returns a boolean value, which will be true when the object is deleted.

So you might think this is safe:

Code: Select all

if(foo && foo->drop())
{
    foo = 0;
}

However, even if your drop() doesn't return true, you should still (in almost all cases) not refer to that object again from the context where you called drop(), since once that reference has been dropped, the object is subject to being dropped from other contexts and being deleted at any time.

After you've dropped your reference, there's no safe way to tell if the object has been deleted or not, so it's never safe to use it again.

This is the way I'd (almost) always do it from userland code.

Code: Select all

if(foo)
{
    (void)foo->drop();
    foo = 0;
}

The (void) before foo->drop() is me being explicit that I don't care about the result, and I want it cleaned off the stack.