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.

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.