Jump to content
  • Advertisement
Sign in to follow this  
deadlydog

Does 'delete' delete an allocated objects allocated objects?

This topic is 4835 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

No it's not a toungue twister, it's my question. I'm using C++ and I've got a class for my game, CShip. Now, I also have another class CShipList which is a linked list that holds CShip objects. Now, when I want to add a new ship to the list I first create a new CShip object using 'new', set it up, then pass a pointer to it to the CShipList's AddShip() function. This function will then create a new linked list item using 'new', and make it's CShip* point to the supplied CShip. Now my question is, when I go to delete that item from the list, can I just delete the linked list item, or do I first need to delete the linked list items CShip*? If all that did was confuse you, here's my functions code. First, this is my structure my linked list uses:
// Items to be held in the List
struct SShipListItem
{
	CShip* cpShip;
	bool bAlive;		// Used to tell if SShipListItem is in DeleteLaterList

	SShipListItem* spNext;
};

This is the CShipLists AddShip() function:
// Add a Ship to the list
// NOTE: The List will use/modify the given Ship. It does not create a new one
bool CShipList::AddShip(CShip* cpShipToAdd)
{
	SShipListItem* spNewItem;

	// Make sure Ship To Add is valid
	if (cpShipToAdd == 0)
	{
		return false;
	}

	// Create the Item
	spNewItem = new SShipListItem;

	// If there are no Items in the List
	if (mspHead == 0)
	{
		mspHead = spNewItem;	// Make the Head point to this item
		mspHead->spNext	= 0;	// Since this is the first item, make it's Next NULL	
	}
	// Else add this Item to the List
	else
	{
		spNewItem->spNext = mspHead;
		mspHead = spNewItem;
	}

	// Set Items data
	mspHead->bAlive	= true;
	mspHead->cpShip	= cpShipToAdd;

	return true;
}

And this is my Delete function, which actually deletes all ships that are marked for deletion:
// Delete all Items in the Delete Later List
void CShipList::DeleteAllShipsInDeleteLaterList()
{
	SShipListItem* spCurrentItem = mspHead;
	SShipListItem* spPreviousItem = 0;

	// Cycle through List and Delete Dead Items
	while (spCurrentItem != 0)
	{
		// If this Item is supposed to be Deleted
		if (!spCurrentItem->bAlive)
		{
			// If we are deleting the first Item in the List
			if (spCurrentItem == mspHead)
			{
				mspHead = mspHead->spNext;			// Move Head to Next Item in List
/*				
				// If the actual Ship exists
				if (spCurrentItem->cpShip != 0)
				{
					delete spCurrentItem->cpShip;	// Delete the actual Ship
				}
*/
				delete spCurrentItem;				// Delete first Item in List
				spCurrentItem = mspHead;			// Make Current Item point to the new Head
			}else
			{	
				spPreviousItem->spNext = spCurrentItem->spNext;	// Make Previous Item point to Next Item
/*				
				// If the actual Ship exists
				if (spCurrentItem->cpShip != 0)
				{
					delete spCurrentItem->cpShip;				// Delete the actual ship
				}
*/
				delete spCurrentItem;							// Delete Current Item
				spCurrentItem = spPreviousItem->spNext;			// Move Current Item to Next Item
			}
		}
		// Else skip to Next Item
		else
		{
			spPreviousItem = spCurrentItem;
			spCurrentItem = spCurrentItem->spNext;
		}
	}
}

I'm wondering if I should be using the commented out code or not of the above in the delete function. The game seems to run fine with or without using the code, I'm just not sure if it's needed or not, as I don't want to have a bunch of allocated memory out there with no pointers to them. I'm also wondering because when I use the same code in my Purge() function I get a windows assertion error and my game kicks out. Here's my Purge() function code:
// Remove all items from both the Ship List and Delete Later List
void CShipList::Purge()
{
	SShipListItem* spCurrentItem = mspHead;
	SShipListItem* spNextItem = 0;
	
	// Cycle through List and Delete all Items
	while (spCurrentItem != 0)
	{
		spNextItem = spCurrentItem->spNext;	// Save Next Item
/*	
		// If the actual Ship exists
		if (spCurrentItem->cpShip != 0)
		{
			delete spCurrentItem->cpShip;		// Delete the actual Ship
		}
*/
		delete spCurrentItem;				// Delete the Ship from the List
		spCurrentItem = spNextItem;			// Move to Next Item
	}

	// Initialize pointers to NULL
	mspHead				= 0;
	mspCurrentShip		= 0;
}

Anybody know if it is needed that I do the extra step or not? Thanks.

Share this post


Link to post
Share on other sites
Advertisement
std::list





In answer to your question, it depends on if it is the "owner" - if it is responsible for creating the ships, then yes it should. Otherwise no.

Share this post


Link to post
Share on other sites
delete will call the object's destructor and then deallocate the object's memory. If the object's destructor deletes pointers it owns then deleting the object will result in the pointers it owns also being deleted. If it doesn't then they won't:
struct OwnsAPointerDoesntDeleteIt
{
int * pointer;
};

struct OwnsAPointerDeletesIt
{
~OwnsAPointerDeletesIt()
{
delete pointer;
}
int * pointer;
};

int main()
{
OwnsAPointerDoesntDeleteIt * object1 = new OwnsAPointerDoesntDeleteIt();
object1->pointer = new int();
delete object1; // object1->pointer is not deleted
OwnsAPointerDeletesIt * object2 = new OwnsAPointerDeletesIt();
object2->pointer = new int();
delete object2; // object2->pointer is deleted
}

Of course when dealing with raw pointers you'll probably need valid copy-constructors and copy assignment operators as well as destructors (the rule of three).
Unless you're doing it for learning purposes I would strongly recommend using std::list over your own homemade list.

Enigma

Share this post


Link to post
Share on other sites
No. If you want to delete extra stuff, add a destructor for the class or structure. It will be invoked right before the object's data gets released.

Share this post


Link to post
Share on other sites
Quote:
Original post by deadlydog
Anybody know if it is needed that I do the extra step or not? Thanks.


Yes, you need to delete pointers to your ships in a for() loop, unless they are deleted by parent's destructor. Also, delete delete's (toungue twister :-> ) pointers only if they're not null, so you don't need to check it yourself.

So, your Purge function should look like this:



// Remove all items from both the Ship List and Delete Later List
void CShipList::Purge()
{
SShipListItem* spCurrentItem = mspHead;
SShipListItem* spNextItem = 0;

// Cycle through List and Delete all Items
while (spCurrentItem != 0)
{
spNextItem = spCurrentItem->spNext; // Save Next Item

delete spCurrentItem->cpShip; // Delete the actual Ship

delete spCurrentItem; // Delete the Ship from the List
spCurrentItem = spNextItem; // Move to Next Item
}

// Initialize pointers to NULL
mspHead = 0;
mspCurrentShip = 0;
}




Btw, why aren't you using STL?


Share this post


Link to post
Share on other sites
Awesome, thanks for all of the replies. As to why I don't just use the STL, I've never actually looked into what all the STL includes and can do. I know it also has a vector class as well, but I've never used it. Anyways, you guys answered my question. I'll just include a destructor in my structs. Thanks.

Dan

Share this post


Link to post
Share on other sites
Quote:
Original post by deadlydog
As to why I don't just use the STL, I've never actually looked into what all the STL includes and can do.

The SC++L (Standard C++ Library, formerly the STL) is quite possibly the best invention since sliced bread (It's definitely between the SC++L, boost::spirit and energy saving lightbulbs, but I haven't quite decided which yet [lol]). You'd be doing yourself a favour to spend a little time learning it.

Enigma

Share this post


Link to post
Share on other sites
Quote:
Original post by Enigma
The SC++L (Standard C++ Library, formerly the STL) is quite possibly the best invention since sliced bread (It's definitely between the SC++L, boost::spirit and energy saving lightbulbs, but I haven't quite decided which yet [lol]). You'd be doing yourself a favour to spend a little time learning it.

Enigma


/agree

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!