dynamic memory allocation using linked lists

[guccimane]

Hey guys, I am trying to learn single linked lists and at first I was having some trouble but now my code works perfectly. At first my program would crash, and when I tried to debug it, it would always crash where ever I had a 'delete' operator within the functions. Such as here:

void add(node* addMe)
	{
		node* newItem=new node;
		newItem=addMe;
		newItem->next=head;
		head=newItem;
		//delete newItem;

	}
	void addNode(int num)
	{
		for(int i=0;i<num;i++)
		{
		node *pointer=new node;
		cout<<"What is the name of family member #"<<i+1<<"?"<<endl;
		cin>>pointer->name;
		cout<<"What is the job title of family member #"<<i+1<<"?"<<endl;
		cin>>pointer->jobTitle;
		cout<<"how old is # "<<i+1<<"?"<<endl;
		cin>>pointer->age;
		cout<<"how much money does this person make ?"<<endl;
		cin>>pointer->salary;
		add(pointer);
		//delete pointer;
		}
	}

So then I comment out the "delete" in these 2 methods, and all of a sudden the code works just fine. But I am confused, as to why my program was crashing when I DID have the 'delete'. I thought everytime I allocated memory using 'new', I am suppose to free the memory using 'delete'.

Here is my complete code that works and compiles just fine.
BUT if you DONT comment out the 'delete' in those 2 functions, then the program will crash. Can someone explain to me why?

#include <iostream>
#include <string>
using namespace std;
struct node
{
	string name;
	int age;
	float salary;
	string jobTitle;
	node *next;
};
class Family
{
private:
	int size;
	node *head;
public:
	Family()
	{
		size=0;
		head=new node;
		head=0;
	}
	~Family()
	{
	}
	void add(node* addMe)
	{
		node* newItem=new node;
		newItem=addMe;
		newItem->next=head;
		head=newItem;
		//delete newItem;

	}
	void addNode(int num)
	{
		for(int i=0;i<num;i++)
		{
		node *pointer=new node;
		cout<<"What is the name of family member #"<<i+1<<"?"<<endl;
		cin>>pointer->name;
		cout<<"What is the job title of family member #"<<i+1<<"?"<<endl;
		cin>>pointer->jobTitle;
		cout<<"how old is # "<<i+1<<"?"<<endl;
		cin>>pointer->age;
		cout<<"how much money does this person make ?"<<endl;
		cin>>pointer->salary;
		add(pointer);
		//delete pointer;
		}
	}




	void printMe(int num)
	{
		
		node *print=new node;
		print=head;

		for(int i=0;i<num;i++)
	{
		cout<<print->name<<endl;
		cout<<print->age<<endl;
		cout<<print->jobTitle<<endl;
		cout<<print->salary<<endl;
		print=print->next;
		cout<<endl;
		
	}
		delete print;
	}
	

};

void main()
{

	Family *familyPointer=new Family;
	
	int num;
	cout<<"how many family members do you have?"<<endl;
	cin>>num;
	familyPointer->addNode(num);
	familyPointer->printMe(num);
	



	delete familyPointer;
	



	system("PAUSE");
}

[subith86]

I thought everytime I allocated memory using 'new', I am suppose to free the memory using 'delete'.

You are right, but you try to use the memory allocated to the linked list after deleting it. This causes the problem. So do a delete only after you are finished playing with the memory you reserved. :)

[subith86]

I will add some other comments on the code.

1. In the constructor, why do you have to allocate memory for head? head is just a pointer to the first block. It doesn't contain any data. Initially head should point to "nothing" since you haven't created any block yet.

Family()
        {
                size = 0;
                head = NULL;
        }

2. In the function add(), you are passing a node pointer for which memory is already allocated at addNode() function. Inside the add() function you are again allocating memory.

void add(node* addMe)
        {
                addMe->next = head;
                head = addMe;
        }

3. In the function printMe() you allocate memory for print. As I see from the code this is supposed to hold only the address and print the values in the block where it is pointing to. So, just a simple pointer is enough.

void printMe(int num)
        {

                node *print;
                print=head;

                for(int i=0;i<num;i++)
                {
                        cout<<print->name<<endl;
                        cout<<print->age<<endl;
                        cout<<print->jobTitle<<endl;
                        cout<<print->salary<<endl;
                        print=print->next;
                        cout<<endl;

                }
        }

4. Make use of the destructor to free the allocated memory for the linked list.

[guccimane]

You are right, but you try to use the memory allocated to the linked list after deleting it. This causes the problem. So do a delete only after you are finished playing with the memory you reserved. :)

Hi, Thanks for the response. Can you elaborate on that a little bit more?
If I dont use 'delete' in those 2 functions, doesn't that cause a memory leak?
And what did you mean by I am trying to use memory allocated to the linked list after deleting it? I thought the entire list was now stored in 'Head', therefore I am free to delete 'newItem' and 'Pointer'.

[Sky Diploma]

Hi,

>>
So then I comment out the "delete" in these 2 methods, and all of a sudden the code works just fine

Firstly I would like to point out that your code is not complete right now. I think it would work for this test-case in main(). However try executing

familyPointer->PrintMe(num);
familyPointer->PrintMe(num);

Now to further understand pointers.

Lets assume that you have a pointer.
A pointer basically holds an address of the variable.
When we dynamically create any variable with the keyword 'new', what we basically ask from the computer is

node* ps;
ps=new node;

The above code has the following internal communication.

Program to Computer
"Hello Computer.. I need some space to store my Variable, Please provide me with some"

The computer says.
"Yeah! Buddy , Here is the address for your extra space .. just store it . "

now lets see this

delete ps;

Program to Computer
"Hey Computer, Thank you for the space. I don't need it anymore !! "

The computer basically reacts and takes back the memory.
We need to be careful of the ps pointer now because it still has the address of the memory space within it.

However ,

delete ps;
ps->doSomething();

THIS conversation is something like this

Program to Computer
"Hey Computer, Thank you for the space. I don't need it anymore !! "

The computer basically reacts and takes back the memory.
--Then when doSomething() is executed;

Computer to Program
"Wait a second!!, You told me you didn't wanna use this space. Why are you messing with it? Something seems wrong *CRASH CRASH CRASH* "

So at the end of it all, we need to understand when, should we ask for new resources and when do we delete it.

and as subith said. When you're done playing with the resource. then tell the computer that you don't need it.

2nd, You don't need to create a new node object every-time you need a pointer. For eg: the function PrintMe allocates dynamic memory and doesn't use it. the worst part is that we don't even delete the memory.

This is the first time I am trying to explain the whole process in a very bizarre but simple manner. I would like to hear comments about this approach . :D

Have a Nice Day

[subith86]

If I dont use 'delete' in those 2 functions, doesn't that cause a memory leak?

Yes it causes. I have posted a second time to your question. See my fourth point in that post. It tells you how to solve.

And what did you mean by I am trying to use memory allocated to the linked list after deleting it?

First look at your addNode() function. You're passing "pointer" to add(). In add(), you make "newItem" point to the memory which is pointed by "pointer" in addNode(). Then you delete "newItem" which means, you deleted "pointer"'s memory. Now the execution in add() is over. It came back to addNode(). Here you are trying to delete "pointer" which was already deleted.

Hope this explanation is good enough for you to understand the problem in your code?? :S

[guccimane]

Thank you very much to both posters who replied!!!!! I understand what I did wrong now. :) Again, thanks for your help.

[subith86]

great !!! please mark the thread as "solved" :)