linked list problem

[TheElder777]

hello all. i've made been a .cpp that is supposed to retrieve 2 lists from user, and then prints either one. this isn't all my code, but it is what i think is wrong. i've been killing my brain cells trying to figure out what is wrong with this but i just can't seem to pull it off. i'm actually disappointed that i had to resort to posting this... but is guess i got to know when to give up.

#include <iostream>
#include <string>
#include <windows.h>

using namespace std;

class emplist // declaration of linked list
{
public:
string name;
int aincome;
int level;

void idqry() //func to print object info
{

cout<<name;
cout.width(20);
cout<<aincome;
cout.width(10);
cout<<level;
cout<<endl;
}
emplist *nxt;
};

emplist *head = NULL; //ini. head pointer for list 1
emplist *curr = NULL; //ini. current pointer for list 1
emplist *head1 = NULL; //ini head pointer for list 2
emplist *curr1 = NULL; //ini current pointer for list 2

int main()
{
char a;
emplist e;
//I ask user to input data for emplist e... i obtain e.name, e.level and blablbla

if (e.level>5)
{
int x = 0;
addem(e, x); //anything wrong here???
}


else
{
int x = 1;
addem(e, x); //or here???
}

cout<<"Would you like to make another add? [Y/N]: ";
cin>>a;


for (curr = head; curr->nxt != NULL; curr = curr->nxt) //attempting to print list 1
{
cout<<"inside for befor idqry"<<endl;
curr->idqry();
cout<<"inside for after idqry"<<endl;
}

return 0;
}


forgot to include addem func... here it is:

void addem(emplist nemp, int x) //addem func
{
if (x = 0)
{
if (curr == NULL)
{
*curr = nemp;
head = curr;
curr->nxt;
}

else
{
curr->nxt = &nemp;
nemp.nxt = NULL;

}
}

else if (x = 1)
{
if (curr1 == NULL)
{
curr1 = &nemp;
head1 = curr1;
curr1->nxt;
}

else
{curr1->nxt = &nemp;
nemp.nxt = NULL;

}
}
else
{
cout<<"CRITICAL ERROR!"<<endl;
}
}


so what do u think??? thanks u!!!

[blackcoder41]

this code does not execute at all so emplist is not initialize.

if (e.level>5)
{
	int x = 0;
	addem(e, x); //anything wrong here???
}


else
{
	int x = 1;
	addem(e, x); //or here???
}

[Galik]

Here is the addem function with what I think is the bugs fixed.

You were writing to the location of a null pointer with this line:

if (curr == NULL)
{
	*curr = nemp; // cur == NULL!! Think you mean curr = & nemp; 

And I think you were failing to ensure the list was terminated in two places:

curr->nxt; // ? Doesn't do anything, think you mean curr->nxt = NULL; 

Here is the code with those fixes:

void addem(emplist nemp, int x)
{
	if (x = 0)
	{
		if (curr == NULL)
		{
			curr = &nemp; // was writing to the location of a null pointer!!
			head = curr;
			curr->nxt = NULL; // Set to null to end the list?
		}

		else
		{
			curr->nxt = &nemp;
			nemp.nxt = NULL;

		}
	}

	else if (x = 1)
	{
		if (curr1 == NULL)
		{
			curr1 = &nemp;
			head1 = curr1;
			curr1->nxt = NULL;
		}

		else
		{	curr1->nxt = &nemp;
			nemp.nxt = NULL; // Set to null to end the list?

		}
	}
	else
	{
		cout<<"CRITICAL ERROR!"<<endl;
	}
}

[Galik]

And I just realised that your addem() function is flawed in another important way.

You are passing your emplist objects in by *value*, which means you are trying to add a *copy* of your emp which is a local variable into your list.

This is no good because your parameter nemp will be destroyed when the addem() function exits!!

You need to pass in emplist pointers like this:

void addem(emplist* nemp, int x) // Note nemp is now a pointer!!

{
	if (x = 0)
	{
		if (curr == NULL)
		{
			curr = nemp;
			head = curr;
			curr->nxt = NULL; 
		}

		else
		{
			curr->nxt = nemp;
			nemp->nxt = NULL;

		}
	}

	else if (x = 1)
	{
		if (curr1 == NULL)
		{
			curr1 = nemp;
			head1 = curr1;
			curr1->nxt = NULL;
		}

		else
		{	curr1->nxt = nemp;
			nemp->nxt = NULL;

		}
	}
	else
	{
		cout<<"CRITICAL ERROR!"<<endl;
	}
}

[Galik]

You would have to modify your main() function to produce emplist objects using new accordingly:

int main()
{
	char a;
	emplist* e = new emplist; // use new to create a new emplist object

	//I ask user to input data 

	if (e->level > 5)  // changed accessor to -> because its a pointer now 
	{
		int x = 0;
		addem(e, x);
	}

	else
	{
		int x = 1;
		addem(e, x); 
	}

	cout << "Would you like to make another add? [Y/N]: ";
	cin >> a;

	for (curr = head; curr->nxt != NULL; curr = curr->nxt) //attempting to print list 1
	{
		cout << "inside for befor idqry" << endl;
		curr->idqry();
		cout << "inside for after idqry" << endl;
	}

	return 0;
}

[TheElder777]

thanks for the quick reply... i've fixed the issues u found.. THANKS FOR THAT. my c.pp is actually running and not crashing at any point, however it seems that both lists will only one node each.. and the data in both of them is identical (it is always the last user input)... do u know what could be causing that? my prof was saying something about double pointers, something about if i wanted to use one function to handle 2 lists... any clue?

[Galik]

I think your output loop looks a bit wrong.

I would have though it would be this:

for (curr = head; curr != NULL; curr = curr->nxt) //attempting to print list 1 

So if the list is empty then head = NULL and nothing will print.

The way you had it will probably not print the last item in the list.

[Galik]

Also your comparison of x is wrong.

When you want to test what the value of something is you need double ='s like this:

if(item == value) {} // GOOD!

if(item = value) {} // bad!! 

A single = is used to SET the value and not compare the value.

Also you needed to update your curr pointer every time you add an emplist object. So the curr pointer is always pointing to the LAST item added like this:

void addem(emplist* nemp, int x) //addem func

{
	if (x == 0) // need double = for comparison
	{
		if (curr == NULL)
		{
			curr = nemp;
			head = curr;
			curr->nxt = NULL;
		}

		else
		{
			curr->nxt = nemp;
			nemp->nxt = NULL;
			curr = nemp; // update current pointer!!

		}
	}

	else if (x == 1) // need double = for comparison
	{
		if (curr1 == NULL)
		{
			curr1 = nemp;
			head1 = curr1;
			curr1->nxt = NULL;
		}

		else
		{	curr1->nxt = nemp;
			nemp->nxt = NULL;
			curr1 = nemp; // update current pointer!!

		}
	}
	else
	{
		cout<<"CRITICAL ERROR!"<<endl;
	}
}

[TheElder777]

yup already fixed some of those, and fixed some extras u found, still getting the same problem...
here is my edited addem func:

void addem (emplist *nemp, int x)
{
if (x == 0)
{
if (curr == NULL)
{
curr = nemp;
head = curr;
curr->nxt = NULL;
}

else
{
curr->nxt = nemp;
curr->nxt = NULL;
curr = nemp;

}
}

else if (x == 1)
{
if (curr1 == NULL)
{
curr1 = nemp;
head1 = curr1;
curr1->nxt = NULL;
}


else
{
curr1->nxt = nemp;
nemp->nxt = NULL;
curr1 = nemp;
}
}
else
{
cout<<"CRITICAL ERROR!"<<endl;
}
}

[Galik]

The version I posted works for me.

You may need to examine the routine you use to print out the contents.

I changed the variable because in your main() function, you use the global curr variable.

That will corrupt your list because the global curr variable is holding important information (where to add new elements).

Here is how I printed the list:

	for (emplist* e = head; e != NULL; e = e->nxt) //attempting to print list 1
	{
		cout << "inside for befor idqry" << endl;
		e->idqry();
		cout << "inside for after idqry" << endl;
	}

[TheElder777]

THANK YOU!!!