A little help with a linked list

[RayvenHawk]

I'm having to create a program that uses a linked list to ask for X & Y points and stores them in a linked list and then displays those results.

My problem here is when it gives me the output I only see the last 2 entries I've made. It's not storing all the values I've inputted. I need a push in the right direction.

Thanks

#include <iostream>
#include <list>

using namespace std;

int main()
{
	struct node
	{	
		int x;
		int y;
		node *nxt;
	};
	node *start_ptr, *temp, *current;
	temp = new node;
	start_ptr = NULL;
	int numPoints = 0;
	int i = 0;

	cout << "Welcome to my Linked List Project" << endl;
	cout << endl;
	cout << "How many points do you wish to enter? ";
	cin >> numPoints;

	do
	{
		cout << "Enter a X value: ";
		cin >> temp->x;
		i++;
		cout << "Enter a Y value: ";
		cin >> temp->y;
		//temp->nxt = NULL;
		start_ptr = temp;
	}
	while (i < numPoints);
	current = start_ptr;
	while(current != NULL)
	{
		cout << "Here are your X values: " << current->x << " ";
		cout << endl;
		cout << "Here are your Y values: " << current->y << " ";
		cout << endl;
		current = current->nxt;
	}
}

[Salem]

You need a new node every time you add a pair of integers from the user input.

[jencas]

There is only one 'new node' in your code, and this is outside the do loop. Does your linked list really contain one node only?? I don't believe that.

[RayvenHawk]

Ok here is my updated code, it won't compile but I can't find where exactly my error is in the code. When I click the error lines it doesnt bring anything up.

This is my source for the class file

#include <iostream>
#include "listnodes.h"

using namespace std;

listnodes::listnodes()
{
	p = NULL;
	g = NULL;
}

void listnodes::addx()
{
	nodex *first, *newNode;
	int num;
	cout << "enter a list of X values ending with -999." << endl;
	cin >> num;
	first = NULL;

	while (num != -999)
	{
		newNode = new nodex;
		newNode->x = num;
		newNode->next = first;

		first = newNode;
		cin >> num;
	}
}

void listnodes::addy()
{
	nodey *second, *newNode;
	int num;
	cout << "enter a list of X values ending with -999." << endl;
	cin >> num;
	second = NULL;

	while (num != -999)
	{
		newNode = new nodey;
		newNode->y = num;
		newNode->Next = second;

		second = newNode;
		cin >> num;
	}
}

void listnodes::displayx()
{
	nodex *current;
	current = first;
	while (current != NULL)
	{
		cout << "X-Value:" << current->x;
		current = current->next;
	}
}

void listnodes::displayy()
{
	nodey *Current;
	Current = second;
	while (Current != NULL)
	{
		cout << "Y-Value:" << Current->y;
		Current = Current->Next;
	}
}

Here is my main program

#include <iostream>
#include "listnodes.h"

using namespace std;

int main()
{

	listnodes ll;

	cout << " Welcome to my linked list program! " << endl;
	cout << " Enter your X value: ";
	ll.addx();
	cout << endl;
	cout << " Enter your Y value: ";
	ll.addy();
	ll.displayx();
	ll.displayy();
}

When I run it, I'm able to input values for both x & y until I use -999. But when I finish with Y the program crashes.

[Salem]

What are p and g in the constructor?

addx and addy seem to be doing the right thing, but they're only updating local variables.
Sure they build a list, but it is lost as soon as you return unless you store the head of the list in a class member variable.

[RayvenHawk]

What are p and g in the constructor?

They are pointers for the nodex & nodey.

#include <iostream>

#ifndef LISTNODES_H
#define LISTNODES_H

using namespace std;

class listnodes
{
private:

	struct nodex
	{
		int x;
		nodex *next;
	}*p;

	struct nodey
	{
		int y;
		nodey *Next;
	}*g;

public:

	listnodes();
	int addx();
	int addy();
	void displayx();
	void displayy();

protected:
                nodex *first;
                nodey *second;
	
};
#endif

addx and addy seem to be doing the right thing, but they're only updating local variables.
Sure they build a list, but it is lost as soon as you return unless you store the head of the list in a class member variable.

How would I go about doing that? I tried using return first & return second, but it wont compile.

[Salem]

Well in addx for example, you would have if ( p == NULL ) p = first; Also, having looked at the code again, it seems to be building the list backwards.

newnode->x = x;
newnode->next = NULL;  // new tail node of the list
thisNode->next = newnode; // add to existing tail
thisNode = newNode; // advance the tail

[RayvenHawk]

Ok nevermind I finally got it to display the values. Instead of trying to seperate each function and then execute them from main, I just threw them all into a single function and executed that from main. Now I have one last problem. It prints them out in reverse order. How would I print them out from 1st inputted to last inputted?

Here is my current working code:

#include <iostream>
#include "listnodes.h"

using namespace std;

listnodes::listnodes()
{
	p = NULL;
	g = NULL;
}

void listnodes::addx()
{
	nodex *first, *newNode;
	nodey *second, *Newnode;
	int num;
	int num2;
	cout << endl;
	cout << "Enter your X values, ending with -999." << endl << endl;
	cin >> num;
	first = NULL;

	while (num != -999)
	{
		newNode = new nodex;
		newNode->x = num;
		newNode->next = first;
		first = newNode;
		cin >> num;
	}
	cout <<endl;
	cout << "Enter your Y values, ending with -999." <<endl << endl;
	cin >> num2;
	second = NULL;

	while (num2 != -999)
	{
		Newnode = new nodey;
		Newnode->y = num2;
		Newnode->Next = second;
		second = Newnode;
		cin >> num2;
	}
	nodex *current;
	nodey *current2;
	current = first;
	current2 = second;
	cout << "The X values are: ";
	while (current != NULL)
	{
		cout <<current->x << " ";
		current = current->next;
	}
	cout << endl;
	cout << "The Y values are: ";
	while (current2 != NULL)
	{
		cout << current2->y << " ";
		current2 = current2->Next;
	}
	cout << endl;
}

Thanks a bunch, especially Salem, you got me thinking about just putting it all into a single function, and viola it works.

[RayvenHawk]

Ok I improved the code to seperate the functions and it works 99% of the intended purpose.

What I'm needing is to sort the X values and leave the Y values unsorted (which is already done). What I current have it just displays the first value I entered, removing the rest.

#include <iostream>
#include <algorithm>
#include "listnodes.h"

using namespace std;

listnodes::listnodes()
{
	p = NULL;
	g = NULL;
}

void listnodes::addx()
{
	nodex *first, *newNode, *sort;
	int num;
	cout << "enter a list of X values ending with -999." << endl;
	cin >> num;
	first = NULL;

	while (num != -999)
	{
		newNode = new nodex;
		newNode->x = num;
		newNode->next = first;
		first = newNode;
		cin >> num;
	}
	if (p == NULL) p = first;
}

void listnodes::addy()
{
	nodey *second, *newNode;
	int num;
	cout << "enter a list of X values ending with -999." << endl;
	cin >> num;
	second = NULL;

	while (num != -999)
	{
		newNode = new nodey;
		newNode->y = num;
		newNode->Next = second;
		second = newNode;
		cin >> num;
	}
		if (g == NULL) g = second;

}

void listnodes::displayx()
{
	nodex *current, *q, *r;
	for( q = p ; q->next != NULL ; q = q->next )
		{
			for( r = q->next ; r->next != NULL ; r = r->next )
				{
					if (r->x<q->x)
						{
							int temp;
							temp = r->x;
							r->x = q->x;
							q->x = temp;
						}
				}
		}

	current = q;
	cout << "Your X values are: ";
	while (current != NULL)
	{
		cout << current->x << " ";
		current = current->next;
	}
	cout << endl;
}

void listnodes::displayy()
{
	nodey *Current;
	Current = g;
	cout << "Your Y values are: ";
	while (Current != NULL)
	{
		cout << Current->y << " ";
		Current = Current->Next;
	}
	cout << endl;
}

What do I have wrong here?

[Salem]

> if (p == NULL) p = first;
Look in your loop.
Your first isn't first anymore.

You're still creating a backwards pointing list (most recent input at the start), with
newNode->next = first;

The other way would be

while (num != -999)
    {
        newNode = new nodex;
        newNode->x = num;
        newNode->next = NULL;
        if ( p == NULL ) {
            p = first = newNode;
        } else {
            first->next = newNode;
            first = newNode;
        }
        cin >> num;
    }

'first' and 'second' should both be called 'current' IMO.

[RayvenHawk]

Ok I made the adjustments to the program so the list now builds itself forward instead of backwards. My only other issue is to sort the X values.

#include <iostream>
#include <algorithm>
#include "listnodes.h"

using namespace std;

listnodes::listnodes()
{
	//sets struct pointers to NULL
	p = NULL;
	g = NULL;
}

void listnodes::addx()
{
	//creates pointers for nodex x and asks for input for int num
	nodex *first, *newNode;
	int num;
	cout << "Enter a list of X values ending with -999." << endl;
	cin >> num;
	first = NULL;

	while (num != -999)
	{
		//stores values from int num into linked list (nodex)
		newNode = new nodex;
		newNode->x = num;
		newNode->next = NULL;

		//function to add values from beginning to end
		if (p == NULL)
		{
			p = first = newNode;
		}
		else
		{
			first->next = newNode;
			first = newNode;
		}
		cin >> num;
	}
}

void listnodes::addy()
{
	//creates pointers for nodes
	nodey *second, *newNode;
	int num;
	cout << "Enter a list of Y values ending with -999." << endl;
	cin >> num;
	second = NULL;

	while (num != -999)
	{
		//sets values from int num into linked list
		newNode = new nodey;
		newNode->y = num;
		newNode->Next = NULL;

		// function to add values from beginning to end
		if (g == NULL)
		{
			g = second = newNode;
		}
		else
		{
			second->Next = newNode;
			second = newNode;
		}
		cin >>num;
	}
}

void listnodes::displayx()
{
	//creates node pointer to print list out
	nodex *current;
	current = p; //sets pointer to equal struct pointer
	cout << "Your X values are: ";
	while (current != NULL)
	{
		//prints values from p until NULL is reached
		cout << current->x << " ";
		current = current->next;
	}
	cout << endl;
}

void listnodes::displayy()
{
	//creates pointer to generate printout
	nodey *current; 
	current = g; //sets pointer to equal struct pointer
	cout << "Your Y values are: ";
	while (current != NULL)
	{
		//displays contents of list until NULL is reached
		cout << current->y << " ";
		current = current->Next;
	}
	cout << endl;
}

Thanks for all your help, it's much appreciated.