list

[evanescence]

Hi, I started writing a program in C++ which uses a list (not doubly-linked) but a problem occured at the very beginning - when I add numbers to the list only the first two are saved.Can somebody help me correct this?

 #include<iostream.h>

struct elem{ int key; elem*next;}
*start=NULL, *p;

void add(int n);
//void check(int n);
void pop();

void main()
{
    int n, br=0;

    cout<<"Vyvedete st-sti za krai natisnete 999"<<endl;

	do{
	   cin>>n;
	   if(n!=999) add(n);
   }

   while(n!=999);
   pop();
   //tyrsene();

}


void add(int n)
{
	
	if(start==NULL)
	{
    
        elem*p=start;
		start=new elem;

		start->key=n;
		start->next=p;

	}

	else if(p->next==NULL)
	{  
		elem*p=start, *q;

		q=new elem;
		q->key=n;
        q->next=NULL;

		while(p->next)
		{p++; };

		p->next=q;
	}
}


void pop()
{
	elem *p=start;
	cout<<"spisykyt e"<<endl;

	while(p)
	{cout<<p->key<<" ";
	p=p->next;
	}
	cout<<endl<<endl;

}

[kbw]

What does this do?

while(p->next)
{p++; };

If you want to walk to the end of the list, you might what to do

while(p->next)
    p = p->next;

[evanescence]

there's no difference when I write it your way

[helios]

Oh, gawd, this is awful.

It's not that only the first two are added. Only the first one is. The program crashes when it tries to add the second.
The problem is this line: else if(p->next==NULL) It may very well be one of the worst things I've ever seen. It's on par with a+b=c.
What happens here is that the compiler rearranges variable declarations so that they are all at the start of the function. This line takes advantage of that fact and tries to dereference a pointer that is not yet set.
Even if the pointer had been set, the comparison makes no sense.
Replace it with just else and apply kbw's correction and it should work. There are still other things that need fixing, though. One of them being that start is a global pointer.

[Duthomhas]

Yes, there is a significant difference between your way and kbws. Yours will always fail.

I believe the difficulty is because you are not using the global variable 'p', but are instead creating and using local 'p's. Give me a second to play with it -- I have more suggestions to improve your program.

[Duthomhas]

OK, here goes.

Line 34 should read p = start;
Line 44 should read

elem* q;
p = start;

Line 42: As helios noted, the condition on line 42 fails after adding the second node (so your program crashes on the third attempt). Just change it to else.


Now for some hints:
You should not be using global variables. Particularly that 'p'. Pass them into your functions instead.

Here is what I suggest:

elem* add( elem* start, int n );
  // This function takes the start of a list of nodes, adds a new node to the end of it, and returns
  // the start of the list. 

You will have to think about how to make your function work this way. Try to use meaningful variable names, and avoid things like 'p' and 'q'.

Now your main function should look like this:

#include <iostream>
using namespace std;

struct elem
  {
  int   key;
  elem* next;
  };

int main()
  {
  int   n;
  elem* start = NULL;

  cout<<"Vyvedete st-sti za krai natisnete 999"<<endl;

  while (true)
    {
    cin >> n;
    if (n == 999) break;

    start = add( start, n );
    }

  // I don't recognize your language, so this is in English...
  cout << "The list you entered is:\n";
  for (enum* node = start; node; node = node->next)
    cout << node->key << endl;

  return 0;
  }

Hope this helps.