Object destroy itself?

[cminusminus]

I made a linked list of dynamic objects, and a function to call them starting from the first one.
I need to destroy a dynamic object using "this" pointer, or some other way...
the problem is that if I call the destructor from inside of the class (some member function), it works, but causes a memory leak, if i call "delete this;" the program crashes.
I'm a still a n00b in c++...

thanks

[matsp]

It is ALMOST never right to call "delete this;".

To find the correct way to do things, you probably would have to post (some of) your code.

--
Mats

[Elysia]

The nodes you allocate using new should be deleted via delete. That simple.
Delete this is dangerous and should only be considered in special situations.
But even better is using smart pointers. That will make sure they get deleted without a memory leak when the time is right.

[cpjust]

It is ALMOST never right to call "delete this;".

To find the correct way to do things, you probably would have to post (some of) your code.

--
Mats

My previous company did that all over the place. Each of their objects contained a Delete() function which just did delete this;
I never saw any problems when using it.
What other situations of delete this; are actually wrong and why?
The only bad use I can think of right now would be trying to delete an object that wasn't dynamically allocated.

[Elysia]

Yep, deleting objects on stack and the problem that it can leave a dangling pointer, if you aren't careful.
I don't think it's that bad, but you should definitely be careful with it. There are some situations where it's helpful.

[laserlight]

For further reading: Is it legal (and moral) for a member function to say delete this?

[cminusminus]

everything exept the deleting works. when I try to call the destructor, it just simply removes the linking to other obj. but does not deallocate the memory.
"delete this;" wont work, and I don't know why...
here's the code of my program:

main.cpp

#include <iostream>

class dyn_obj
{
public:
	dyn_obj();
	~dyn_obj();
	void function(int &);
	
	dyn_obj *prev, *next;

private:
	bool del_now;
};

dyn_obj *first_obj = 0, //first object's mem address
		*Prev_obj = 0; //the current object's address

dyn_obj::dyn_obj():del_now(false)
{
	
	next = 0;//set the link to the next object to 0, there is no next object yet
	if( first_obj == 0 )//if this is the first object in the list
	{
		first_obj = this;//set this as the first object
		prev = 0;//set prev to 0, this are no previous objects.
	}
	else//if NOT the first object
	{
		prev = Prev_obj; //link this object to the previous object
		prev -> next = this;//link the previous object to this object
	}
	Prev_obj = this;
}

dyn_obj::~dyn_obj()
{
	if(next == 0)
	{
		if(prev == 0)//if this is the only object existing
		{
			first_obj = 0; //set first object to 0, no first object
		}
		else//if this is the last object in the list
		{
			prev -> next = 0; //remove the linking from the object before
		}
	}
	else
	{
		if(prev == 0)//if this is the first object in the list
		{
			first_obj = next; //set the next object as the first one
			next -> prev = 0;
		}
		else//if this object is somewhere in the middle of the list
		{
			prev -> next = next;//link the previous object to the next one
			next -> prev = prev;//and vice versa
		}
	}
}

void dyn_obj::function(int & value)
{
	value++;
	
	if(del_now)delete this;
	else del_now = true;
}

int call()
{
	int n = 0;
	if( first_obj != 0 )//if the list is not empty
	{
		dyn_obj *object = first_obj;//create a pointer to the first object in the linked list
		while( true )
		{
			object -> function(n);//call the object
			if(object -> next != 0) object = object -> next;//if not last object, move to the next one
			else break;
		}
	}
	return n;
}


int main()
{
	for(int n = 0; n < 10; n++) new dyn_obj;
	std::cout << "10 objects created. let's call them." << std::endl;
	std::cout << "the function \"function\" was called " << call() << " times." << std::endl;
	std::cout << "let's call the functions a second time, making the objects delete themselves." << std::endl;
	call();
	std::cout << "deleted.";
	std::cin.get();
}

[Elysia]

This implementation will confuse and creates bugs. That's all.
Reconsider it to a more true OOP implementation.

[matsp]

I would personally like to see a "holder" (aka container) object for your dyn_obj's - this would have the pointers "first_obj" and "prev_obj" [which traditionally are called head and tail respectively, but that's your choice - your names are certainly better than "a" and "b"!].

Then let the holder deal with deleting the objects.

Your current construction makes the whole thing a bit convoluted.

I don't see any reason why the current design doesn't de-allocate the memory, however.

--
Mats

[vart]

Not to mention - that removing node from the middle of the 2-linked list requires to update 4 pointers and not 2

[cminusminus]

yes, i'd like to make an another class where to place that call() function, the *first_obj and *prev_obj pointers and make it "parent" to the dyn_obj class, but when I tried I got like 40 errors, and posted that one, I still need to learn the class inheritance and some other important stuff.

[matsp]

There should be no need for inheritance for such a class - one class CONTAINS the other class, but they are not related, just like the class below is not inheriting "string":

class person
{
   string name;
...
  public:
   const string &getName() { return name; }
...
}

--
Mats

[cminusminus]

Not to mention - that removing node from the middle of the 2-linked list requires to update 4 pointers and not 2

4? two of them will be destroyed...
ok it's always good practise

[indigo0086]

For further reading: Is it legal (and moral) for a member function to say delete this?[/url]

Oh you silly programmers with your hyperboles.

[Elysia]

I would like to see something along the lines of this:

template<typename Type> class CLinkedList
{
public:
	//CLinkedList(): pNext(NULL), pPrevious(NULL), pObject(NULL), pBeginOrEnd(NULL) { }
	CLinkedList(CLinkedListBeginOrEnd<Type>* pBeginOrEnd): pNext(NULL), pPrevious(NULL), pObject(NULL), pBeginOrEnd(pBeginOrEnd) { }
	//CLinkedList(const CLinkedList<Type, Allocator>& rCopy);
	void InsertAfter(CLinkedList<Type>* pAfter);
	void InsertBefore(CLinkedList<Type>* pBefore);
	void InsertAtBeginning();
	void InsertAtEnd();
	void Remove();
	//void Initialize(CLinkedListBeginOrEnd<Type, Allocator>* pBeginOrEnd);
	//CLinkedList& operator = (const CLinkedList<Type, Allocator>& rCopy);
	Type* pObject;

private:
	CLinkedList* pNext;
	CLinkedList* pPrevious;
	CLinkedListBeginOrEnd<Type>* pBeginOrEnd;
};

CLinkedListBeginOrEnd is optional, but is useful is you want your lists to have a head and tail pointer.
Implementation is missing on purpose.
Perhaps you might get an idea of what is possible via linked lists in C++...