Linked List Program

[romyboy2k9]

Hello, could some please help me with a current project that I'm working on for school? I have the following code, but the output shows unknown address. The following is the description of my assignment, and the code I have so far. Thank you.

Write a program that creates a linked list of points. For this Assignment we define a point as a set of integers, X and Y. This linked list should insert the points ordered by the x value. The user is prompted to enter as many points as they desire. Each point is a new node in the linked list. After the user has entered all the points they wish to enter, the program is to output the linked list data (i.e. X and Y integers) as it appears in the linked list.

#include <iostream>
#include <list>
#include <cassert>
#include <ostream>

using namespace std;

struct node
{
	node *info;
	node *link;
	int x, y; 
};

int main()
{
	node  *first, *last, *newNode;
	int input;

	cout << "Enter the interger for x and y value" << endl;
	first = NULL;
	last = NULL;

	do 
	{
		//x integers
		cout << "Input X value or -999 to stop: ";
		cin >> input;//read info
		int x = input; //store info (x) into input

		newNode = new node();//allocate memory for the type node
		assert(newNode != NULL);//terminate program is no memory space

		newNode->x = input;//copy value into input
		newNode->link = NULL;

		if (first == NULL)
		{
			first = newNode;
			last = newNode;
		}
		else
		{
			last->link = newNode;
			last = newNode;
		}
	
		
		//y integers
		cout << "Input Y value or -999 to stop: ";
		cin >> input;//get info
		int y = input;//store in for (y) into input

		newNode = new node;//allocate memory for the type node
		assert(newNode != NULL);//terminate program is no memory space

		newNode->y = input;//copy value into input
		//newNode->link = input;
		newNode->link = NULL;
		
		if (first == NULL)
		{
			first = newNode;
			last = newNode;
		}//end if
		else
		{
			last->link = newNode;
			last = newNode;
		}//end else	
	}//end do
	while (input != -999);
	

	newNode = first;
	while (newNode != NULL)
	{
		//cout << "(" << newNode ->x << "," << newNode ->y  << ")" <<endl;
		
		// Test output
		cout << newNode ->x << endl;
		cout << newNode ->y << endl;
		newNode = newNode->link;
	}
	system ("pause");
}

[thelamb]

It really makes sense what you're doing wrong... I think you basically wrote the code that sents newNode->x and then copy/pasted it, changing 'x' to 'y'.

Look at the things that you are doing for _both_ x and y, and think about if it makes sense...

Let me state it a little differently:
You do:
-> Ask user for X
-> Create newNode
-> Initialize newNode->x
-> Ask user for Y
-> Create newNode
-> Initialize newNode->y

What if you would do this:
-> Ask user for X
-> Ask user for Y
How would you continue this? Create two newNode's or one?

[arkoenig]

Your struct node has a data member named info that you define but never use. What purpose does it serve?

[romyboy2k9]

Well, after a few hours of going through the code and back, I was able to display the output that I think is intended. I tried the idea of creating two newNodes and storing the y data in it. But the code is still kind of strange because it outputs the x node along with the value from the y input. However it works. I'm kind of confused. I'm not sure how to explain it so take a look at the code:

#include <iostream>
#include <list>
#include <cassert>
#include <ostream>

using namespace std;

struct node
{
	node *link;
	int infoX, infoY;
};

int main()
{
	node  *first, *last, *newNodeX, *newNodeY;
	int x, y; 

	cout << "Enter the interger for x and y value" << endl;
	first = NULL;
	last = NULL;
	cout << endl;
	do 
	{
		//x integers
		cout << "Input X value or -999 to stop: ";
		cin >> x;//get x value
		int infoX = x; //store info (x) into info

		newNodeX = new node();//allocate memory for the type node
		assert(newNodeX != NULL);//terminate program is no memory space

		newNodeX->infoX = x;//copy value into info
		newNodeX->link = NULL;

		if (first == NULL)
		{
			first = newNodeX;
			last = newNodeX;
		}
		else
		{
			last->link = newNodeX;
			last = newNodeX;
		}
	
		
		//y integers
		cout << "Input Y value or -999 to stop: ";
		cin >> y;//get y value
		int infoY = y; //store info (y) into info

		newNodeY = new node;//allocate memory for the type node
		assert(newNodeY != NULL);//terminate program is no memory space

		newNodeY->infoX = y;//** y value is copied to x info **//
		newNodeY->link = NULL;
		
		if (first == NULL)
		{
			first = newNodeY;
			last = newNodeY;
		}//end if
		else
		{
			last->link = newNodeY;
			last = newNodeY;
		}//end else	
	}//end do
	while (x && y != -999);
	
	cout << endl;
	newNodeX = first;

	// Outputs the X info with y value
	while (newNodeX != NULL)
	{
		cout << newNodeX ->infoX << endl;
		newNodeX = newNodeX->link;
	}
	system ("pause");
}

[thelamb]

" I tried the idea of creating two newNodes and storing the y data in it. "

That's not what I meant... you were already creating two newNodes in your original solution, and I tried to make you think about _why_ you do that.

Just think logically, you have one structure that has a place to store infoX and infoY _together_. So when you ask the user to input X and Y, why do you want to store this in 2 different structures instatiations?

What I just realized is that in your original solution you defined one newNode pointer, and just called newNode = new node() twice... And now you changed it to newNodeX and newNodeY, I missed that in my first post, I thought you already had a newNodeX and newNodeY.

In any case, I think you don't understand the use of the 'new' operator.

int* pInt;   // pointer to an int variable
int* pInt2;  // Idem.

// Both pInt and pInt2 are 'lose' pointers, they are totally useless at the  moment.
// So let's make them useful:
pInt  = new int;
pInt2 = new int[2];
// Now the new operator has allocated memory where pInt and pInt2 will be stored.
*pInt = 5;   // 'value-pointed-by' pInt is now set to 5
pInt2[0] = 10; 
pInt2[1] = 15; 

// Because we've dynamically created the storage for pInt and pInt2, we also need to free it:
delete pInt;
delete[] pInt2; // pInt2 is an array, so make sure that we delete an array.

// Never, ever write code like this:
int* pInt3;
pInt3 = new int;
*pInt3 = 3;
pInt3 = new int; // ALARM! you've just created new memory to store pInt3, without first freeing the memory that you originally allocated (that now stores 3)

// Also, never ever forget to delete something that you created with new.
delete pInt3;
//delete pInt3; // Also, never ever delete the same pointer twice!

I hope it clears things up for you that you can allocate a newNode just once, and store _both_ infoX and infoY in the same newNode structure.

[romyboy2k9]

Ok, that made sense, and I just want to say that I really appreciate this help. However, I'm not sure if I'm still doing this right. I made a little adjustment to the code as the following.

#include <iostream>
#include <list>
#include <cassert>
#include <ostream>

using namespace std;

struct node
{
	node *link;
	int infoX, infoY;
};

int main()
{
	node  *first, *last, *newNode;
	int x, y; 

	cout << "Enter the interger for x and y value" << endl;
	first = NULL;
	last = NULL;
	cout << endl;
	do 
	{
		//x integers
		cout << "Input X value or -999 to stop: ";
		cin >> x;//get x value
		int infoX = x; //store (x) into infoX

		newNode = new node();//allocate memory for the type node
		assert(newNode != NULL);//terminate program is no memory space

		newNode->infoX = x;//copy value into info
		newNode->link = NULL;

		if (first == NULL)
		{
			first = newNode;
			last = newNode;
		}
		else
		{
			last->link = newNode;
			last = newNode;
		}
	
		
		//y integers
		cout << "Input Y value or -999 to stop: ";
		cin >> y;//get y value
		int infoY = y; //store (y) into infoY

		newNode = new node;//allocate memory for the type node
		assert(newNode != NULL);//terminate program is no memory space

		newNode ->infoX = infoY; // both x and y value points to newNode
		newNode->link = NULL;
		
		if (first == NULL)
		{
			first = newNode;
			last = newNode;
		}//end if
		else
		{
			last->link = newNode;
			last = newNode;
		}//end else	
	}//end do
	while (x && y != -999);
	
	cout << endl;
	newNode = first;

	// Prints output
	while (newNode != NULL)
	{
		cout << newNode ->infoX << endl;
		newNode = newNode->link;
	}
	system ("pause");
}

[thelamb]

No... you're still doing _exactly_ what I said you should 'never ever' do...

I really don't know how I can make it more clear other than writing the code for you, which defeats the point of you learning how to do it.

Do you know that this is possible:

int xValue = 5;
int yValue = 6;

node* newNode = new node();
newNode->infoX = xValue;
newNode->infoY = yValue;

delete newNode; // Don't forget to free the memory once you are done with it!!

[romyboy2k9]

Thank you thelamb. ok now I get what you're saying. And it actually simplified my code more.

#include <iostream>
#include <list>
#include <cassert>
#include <ostream>

using namespace std;

struct node
{
	node *link;
	int infoX, infoY;
};

int main()
{
	node  *first, *last, *newNode;
	int x, y; 

	cout << "Enter the interger for x and y value" << endl;
	first = NULL;
	last = NULL;
	cout << endl;
	do 
	{
		//x integers
		cout << "Input X value or -999 to stop: ";
		cin >> x;//get x value

		//y integers
		cout << "Input Y value or -999 to stop: ";
		cin >> y;//get y value

		int infoY = y; //store (y) into infoY
		int infoX = x; //store (x) into infoX

		node* newNode = new node();//allocate memory for the type node
		assert(newNode != NULL);//terminate program is no memory space

		newNode->infoX = x; //copy x value into infoX
		newNode->infoY = y;	//copy y value into infoY

		newNode->link = NULL;

		if (first == NULL)
		{
			first = newNode;
			last = newNode;
		}
		else
		{
			last->link = newNode;
			last = newNode;			
		}
	
	}//end do
	while (x && y != -999);
	
	cout << endl;
	newNode = first;

	// Prints output
	while (newNode != NULL)
	{
		cout << "The x and y values are: ";
		cout << "(" << newNode ->infoX << "," << newNode ->infoY  << ")" <<endl <<endl;
		newNode = newNode ->link;


	}
	system ("pause");
}

[thelamb]

yay :D

Now get rid of that 'system("pause")' call... check the sticky's on this board for information about how to 'properly' pause the execution of your program.