Deleting itself ok?

I was just wondering, can an object (from a class) delete itself? The code below seems to compile fine, but I wanted to be sure. I do not access any of the member variables after the deletion (the NextNode is stored in a temporary variable), but I'm still doing operations in the objects method (member funtion).
Is this ok to do?

Code:

//+-----------------------------------------------------------------------------
//| Deletes a node from the list
//+-----------------------------------------------------------------------------
VOID GROUP::DeleteNode(GROUP** PreviousNextNode)
{
   //Data
   GROUP* TempNode;

   //Stop if NULL is passed
   if(PreviousNextNode == NULL) return;

   //Delete the current node
   TempNode = NextNode;
   SafeDelete(*PreviousNextNode);
   (*PreviousNextNode) = NextNode;
}

PreviousNextNode is a pointer to the previous node's NextNode pointer (!).

>I was just wondering, can an object (from a class) delete itself?

Not entirely sure I follow you. If the following is true:

if (*PreviousNextNode == this)

then answer is no, even though it may be appear to work sometimes, it would be unsafe. However, if *PreviousNextNode is different instance to 'this', there should be no problem.

I had similar situation a while ago, where I had a timer object which called a function after an interval. This call back function would then delete the timer object which called it. The point being, when the function completes, where does it return to? Amazingly the code seemed to work most of the time, but work go crash every so often. In the end, I had to use a different approach.

You can say:

delete this;

...sure. Here's a self extraction:

Code:

void Node::die(){
if(next)
 next->prev = prev;
if(prev)
 prev->next = next;
delete this;
}


//...eg:


void List::something(){
//...
if(node->value < 10)
 node->die();
}

Still, you would better off just doing the extraction in the destructor:

Code:

~Node::Node(){
if(next)
 next->prev = prev;
if(prev)
 prev->next = next;
}

//...

void List::something(){
//...
if(node->value < 10)
 delete node;
}

Both accomplish the same thing, but in my opinion, the second is cleaner.

Code:

void Node::die(){
if(next)
 next->prev = prev;
if(prev)
 prev->next = next;
delete this;
}

What happens if the Node instance is on the stack?

Well, then the results would be undefined. But almost by definition, a linked list is going to dynamically allocate it's nodes.

It is possible then? Thanks!

>Is this ok to do?
Yes, if you're careful about it. Objects are often written so that they can commit suicide.

-Prelude

You should never have
delete this
It's much better to break encapsulation than to have the
confusion resulting from this kind of code.

Originally posted by Nick
You should never have
delete this
It's much better to break encapsulation than to have the
confusion resulting from this kind of code.

Yes you should. There are certain situations which are simplified by using delete this;
Using named constructors you can ensure the object is created dynamically; and deleted correctly.

http://users.utu.fi/sisasa/oasis/cpp...ore-mgmt.html#[16.14]

I cannot think of any *properly* designed data structures
where you would delete this. For a linked list you might
have the nodes delete them selfs recursivly. That's really
ugly.

Originally posted by Nick
http://users.utu.fi/sisasa/oasis/cpp...ore-mgmt.html#[16.14]

I cannot think of any *properly* designed data structures
where you would delete this. For a linked list you might
have the nodes delete them selfs recursivly. That's really
ugly.

So how do you do a linked list then, without some external node handling functions? I used to have an extra object as a first node, and always looking one step ahead (meaning that the data in node 2 was actually the data for node 1), but that is kinda ugly from a programming perspective.

[16.14] Is it legal (and moral) for a member function to say delete this?

As long as you're careful, it's OK for an object to commit suicide (delete this).

Here's how I define "careful":

1.You must be absolutely 100% positive sure that this object was allocated via new (not by new[], nor by placement new, nor a local object on the stack, nor a global, nor a member of
another object; but by plain ordinary new).
2.You must be absolutely 100% positive sure that your member function will be the last member function invoked on this object.
3.You must be absolutely 100% positive sure that the rest of your member function (after the delete this line) doesn't touch any piece of this object (including calling any other
member functions or touching any data members).
4.You must be absolutely 100% positive sure that no one even touches the this pointer itself after the delete this line. In other words, you must not examine it, compare it with another
pointer, compare it with NULL, print it, cast it, do anything with it.

Naturally the usual caveats apply in cases where your this pointer is a pointer to a base class when you don't have a virtual destructor.

Most of these seems to fit my approach, but can someone explain 2). Does it mean I can't call any other methods (which is kinda obvious)? But isn't that what 3) says?

I think the emphasis on OK is supposed to mean
you can delete this and not core dump. It
doesn't mean that it's in good style.

So how do you do a linked list then, without some external node handling functions?

The code will amount to something like this

Code:

List::~List() 
{
       Node* curr = headptr;
       while(curr != 0) {
               Node* t = curr->next;
               delete curr;
               curr = t;
        }
}

2.You must be absolutely 100% positive sure that your member function will be the last member function invoked on this object.

They are talking about having delete this in a suicide member function not the destructor.

Originally posted by Nick
The code will amount to something like this

Code:

List::~List() 
{
       Node* curr = headptr;
       while(curr != 0) {
               Node* t = curr->next;
               delete curr;
               curr = t;
        }
}

If I got that right, List is a class handling the list with nodes of the type Node. What I want is a linked list which handles itself, without any external class/function/anything.

Well, although you can use just a list object without a node object, you will find the complications which arise from the inherent distinctions between the two make the drawbacks outweigh the benifits. For one, the list destructor should call delete_list(). The node might extract itself in it's destuctor. How will they know the difference? Also, there are operations on nodes which are clearly not list operations and vice-versa. Finally, separating the two means that although the node needs only three fields (data, prev, and next), you have a great opportunity to add arbitrary fields to a list object without adding the bulk to objects which should rightfully just be nodes. If you'd like a fairly robust List template, just PM me and I'd be glad to send it to you to play with.

Never figured out what's so great about linked lists in the first place. Can't see a need for them in C++, although there may be with C.