delete called in class

Member
Posts: 41
Joined: 2006.01
Post: #1
Here's a question that may interest some here:

I have sometimes called delete() from within the object being deleted like this:

void projectile::draw()
{
// move the thing
// detect hits
if(hit)
{
handle hit
delete this;
return; //immediately
}
}


I've done this several times over the years, and I find that it ports and functions correctly as long as I (obviously) remember to take care while handling linked lists etc.

The issue of 'delete this' doesn't occur in any textbooks I have, and I've never seen it in any code anywhere. Is it considered bad coding? I have found it handy and simple, and as long as functions that loop through lists are defined in the class, I don't see a problem with it.
Quote this message in a reply
Moderator
Posts: 365
Joined: 2002.04
Post: #2
It's OK for objects to delete themselves providing you're careful:
  • Make sure you don't access any methods or member variables of the affected object between using delete and the end of the method you put it in.
  • Be careful that you don't do anything else which could implicitly cause the deleted object to be accessed after it has already been deleted.
  • Don't allow objects of this class to ever be created on the stack, or you'll crash when the object goes out of scope. Maybe make all constructors private and provide public static methods to create objects.
  • You'll probably want to do something in the destructor to make sure the object removes itself from anything that contains it, to prevent yourself from leaving trailing pointers behind.

I've certainly done it before, but I find it makes the code slightly more difficult to understand and it's potentially a source of bugs. If you can keep track of it and code safely, it's probably fine.

Neil Carter
Nether - Mac games and comic art
Quote this message in a reply
zKing
Unregistered
 
Post: #3
I completely agree with NCarter. This is legal, I've seen it used; it has its place. It also can be dangerous. But be aware that the bugs caused by defects around this idiom can be a bear to find.

Remember that deleting an object twice or accessing a deleted object's members has undefined behavior. One really big point I like to underscore are the words "undefined behavior". This means that _anything_ can happen... and it doesn't have to behave the same way in any kind of repeatable manner. Each compiler, heck each build from a compiler, each run of each build can do something entirely different... including seeming to run just fine.

Oh, and I'd recommend taking full advantage of the fact that "delete 0;" is defined to have no effect and dereferencing a null pointer will get you an immediate reaction Wink. NULL any pointers that have large visible scope after deleting them. At least you get _defined_ (and portable) behavior with NULL's.
Quote this message in a reply
Moderator
Posts: 3,577
Joined: 2003.06
Post: #4
Add me to the agree column. delete this; is okay with caution. The only reason I'm adding in here is to add emphasis to zKing on the NULL thing. Any freed pointer, including objects should be set to NULL immediately afterwards for good measure. That has saved me countless hours debugging over the years. It's not quite so bad now with the protected memory in OS X, but dangling pointers could really cause some truly bizzarre behavior back in the Classic days, and finding that bug after the first hard restart was always easier if it was explicitly set to NULL or even 0xdeadbeef rather than some trivial memory address that could hide in the crowd.
Quote this message in a reply
DoG
Moderator
Posts: 869
Joined: 2003.01
Post: #5
Delete this is indeed dangerous because you might leave around dangling pointers somewhere else. I've never had to call "delete this" in all my programming, and I am inclined to think it points (no pun intended) to bad design.
Quote this message in a reply
Sage
Posts: 1,199
Joined: 2004.10
Post: #6
I'm with DoG -- as far as I'm concerned, it may very well be legal, but I wouldn't touch it with a 10 foot clown pole.

In the situations where I needed something like it, what I've done is made a deletion queue in my game loop, where objects which want to go away ( say, single-shot stuff like explosion particle systems, sounds, etc ) can enqueue themselves. At the end of the current loop, they're all deleted. Thus there's no complex scoping issues.

In fact, this is useful enough that my base Entity class has the method autoDelete() which takes care of this. Plus, an virtual aboutToBeDeleted() method allows the objects to remove themselves from global collections, scenegraphs, etc. This is important since when dealing with multiple inheritance, when objects are being deleted, 'this' isn't always the same based on the current destructor.

The only place where this -- even when dealing with my autoDelete -- is safe is when an object is a fire and forget.

An interesting side note is that this make dismissal of windows in my GUI to be very clean. The callback for a "close" button can simply call autoDelete(). Works like a champ.
Quote this message in a reply
Member
Posts: 41
Joined: 2006.01
Post: #7
I disagree because:

(1) Multiple references to an object pose the same hazards no matter where delete is called.

(2) In cases that are "fire and forget" as in the example, the references to an object can be confined entirely to the class. I would say that in such cases, 'delete this' provides good form and function since it's highly legible and fast.

It seems to me that the one and only issue with 'delete this' is that the calling function has to be aware that the list can change each iteration; and that function can be (should be) made a static member of the class. There are many ways to change a list in mid-iteration, I've also written code that moves objects to the head or tail as the list is being cycled through, and that poses the same hazards.
Quote this message in a reply
DoG
Moderator
Posts: 869
Joined: 2003.01
Post: #8
I don't see any reason why objects should delete themselves. It kinda violates object boundaries, as the object then has to know about all external references that need to be informed of its deletion. It's hard to create lean and clean code if everything is interdependent.

Deletion should be performed by another object that does keep track of such information. It might be the container class, or a controller, or whatnot.
Quote this message in a reply
Member
Posts: 131
Joined: 2004.10
Post: #9
My point of view, who ever created the object, destroys the object. An object doesn't create itself so it really shouldn't commit suicide. A better way to handle this would be for the object to mark itself for termination and then have the creator deal with the clean up. This way you know exactly when and where an object dies instead of having to guess.

Not to mention that if you subclass this object this delete(this) code will probably give you a headache.
Quote this message in a reply
Member
Posts: 304
Joined: 2002.04
Post: #10
>>
My point of view, who ever created the object, destroys the object
<<
I disagree. If a gun object creates a bullet object and sends it flying away, it shouldnt be the responsibility of the gun object to delete it when it hits a wall.

Im sure there are better ways but what I do is keep a member boolean called shouldDelete which gets inited to false. When a game object wants to delete itself it sets it to true. At the end of my game loop I run through the game objects and delete anything where shouldBeDeleted() returns true. If Im using a stl list then I use the remove_if method.
Quote this message in a reply
Member
Posts: 198
Joined: 2005.01
Post: #11
I'd like to point at my RefPtr/RefCnt classes as a perfectly good example of where 'delete this' makes sense:

http://svn.allusion.net/svn/kos/tiki/inc...i/refcnt.h
http://svn.allusion.net/svn/kos/tiki/src...object.cpp (top part)

Basically you derive your class from RefCnt and it becomes automatically reference counted. You then use RefPtr<Type> to contain a pointer to it. The RefPtr just calls addref and unref automatically on construction/destruction (respectively). RefCnt implements those and calls 'delete this' when it reaches zero.

I've used these things very extensively in several projects and it's absolutely fabulous. Almost as good as having garbage collection.

Cryptic Allusion Games / Cryptic Allusion, LLC
http://www.cagames.com/
Quote this message in a reply
DoG
Moderator
Posts: 869
Joined: 2003.01
Post: #12
Reference counting just delegates memory management issue, an object still wouldn't refcount itself. You need to have something external that tells it when to stop existing.
Quote this message in a reply
Member
Posts: 198
Joined: 2005.01
Post: #13
While that's true, the class still uses "delete this" and I thought it made a good example of where that's ok. The external something doesn't really tell it to stop existing, it just tells it that it's not referenced as much anymore. In the end the class itself has to realize there are no references and delete.

Cryptic Allusion Games / Cryptic Allusion, LLC
http://www.cagames.com/
Quote this message in a reply
Member
Posts: 131
Joined: 2004.10
Post: #14
codemattic Wrote:I disagree. If a gun object creates a bullet object and sends it flying away, it shouldnt be the responsibility of the gun object to delete it when it hits a wall.
You can argue a design decision any way you like.

You could also attack it as such...

A gun object asks a scene graph (or whatever you use for managing all the objects) for a bullet object. Sets the bullets params and sets it on it's way. The bullet, once it reaches end of life sets a flag stating, Ok I'm dead, kill me now. The scene graph, when it reaches the bullet, will see, Oh a dead object, time to clean up. Here, the scene graph handles birth and death.

But like anything, this can be debated with religious furor so how ever you like to code something then might as well stick with it.
Quote this message in a reply
Member
Posts: 198
Joined: 2005.01
Post: #15
The thing I've read (and which I tend to agree with) is that if you're starting to talk about object ownership and responsibility, it's probably time to get a reference counting scheme Wink

(whether it's the wacky delete this version I presented, or something else...)

Cryptic Allusion Games / Cryptic Allusion, LLC
http://www.cagames.com/
Quote this message in a reply
Post Reply