Jump to content
  • Advertisement
Sign in to follow this  
Chrono1081

Memory leak question part 2

This topic is 3547 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

First off, thank you guys for the great suggestions in my previous thread about this. Now for my next question. I think I solved the memory leak, but I'm not really sure I solved it. (I'm still trying to grasp linked lists). Here is the function I have in question: *Note: front is a pointer of "Node" type.
//Removes a node from the list
int LinkedList::RemoveFront()
{
	int returnVal;
	Node *temp;

	if(front != NULL)
	{
		//list is not empty, return the first node
		returnVal = front->data;
		temp = front->link;  //<- Look here
		delete front;  // <- And here
                front = temp;
	}

	else 
	{
		//list is empty, return 0
		returnVal = 0;
	}

	return returnVal;
}

On the part marked "Look here" I stored the link into "temp", then I deleted the information in "front" and then assigned front what was in temp. From my point of view it looks like I have no more memory leak. Is this correct? Or am I missing something?

Share this post


Link to post
Share on other sites
Advertisement
This should be correct, though I don't see much point in returning the front item (how would you distinguish between 0 that was the value of the front node and 0 that means list is empty?)

Share this post


Link to post
Share on other sites
I also do not think you need to return in this. Although it is sometimes expected that popping (removing) the front or back will return an element, this turns out to be hazardous for a few reasons.

For one, you could write a container of a rather large type. This means that you have to copy it to the stack when it's popped from the front, or you have to default-construct one, then copy. For trivial types like int char and so on, this is also trivial, but for the rather large type, it's an unexpected inefficiency.

Note that usually, the user gets a reference to the value at the front or back through a peek function, or through iterators.

Also, as mentioned, how do you know whether the value returned is valid? Well, one solution is to not have it return an int, but a bool on whether it's valid or not. The object will actually be returned through a reference parameter. You could also supply a second version that doesn't take an int& if the user doesn't want to supply one. (And defaulting reference arguments is not usually good.)

Lastly: the local variable temp should be defined closer to where it's used. As is, your compiler will be forced to put temp on the stack, even if it's never used. (Although it might optimize it away, that's another story.) On the other hand, if you find it is really more clear and intuitive defined where it is, that's fine as the code isn't really THAT bad, THIS time, because it's a basic type.

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!