deleting and displaying in a linked list

[tracethepath]

i am making a menu driven program on linked lists.
the program's display function is printing garbage values.
i.e if n=3...i add three values and when the values are displayed the last entry is displayed and the rest two enteries are garbage values
for eg.

MENU (Linked List)

1. CREATE
2. INSERT
3. DELETE
4. DISPLAY
5. EXIT

Enter Your Choice: 1

Enter The Number Of Members You Want To Add: 3

Enter Name: sbi
Enter Roll: 44

Enter Name: chi
Enter Roll: 65

Enter Name: csc
Enter Roll: 11

MENU (Linked List)

1. CREATE
2. INSERT
3. DELETE
4. DISPLAY
5. EXIT

Enter Your Choice: 4

Current List:
NAME: csc
ROLL: 11

NAME: ☻∙├H.;♠╜▲├QR♠3╔.ï0►▲↓
ROLL: -29906

NAME: ┤H═!;┌r♂┤H═!r♣ASP
ROLL: 20563

and when choice 3 is used the program doesnt come to an end...
i will be gratefull if somebody helps me out...

struct student
{
  char name[15];
  int roll;
  student*next;
};

void display(student*first)
{
  student*ptr;
  ptr=first;
  cout<<"\nCurrent List:\n";
  while(ptr!=NULL)
  {
    cout<<"NAME: "<<ptr->name<<"\nROLL: "<<ptr->roll<<"\n\n";
    ptr=ptr->next;
  }
}

void delet(student*first, int val)
{
  student*ptr,*list;
  ptr=first;
  while(ptr!=NULL)
  {
    if(ptr->roll==val)
    {
      list=ptr;
      ptr=ptr->next;
      delete list;
    }
  }
}


void main()
{
  student*first, *last, *x;
  first=last=NULL;
  int c=0, val, choice, n, i;
  char ch='y';

  while (c==0)
  {
    clrscr();
    **menu**
    cout<<"\n\nEnter Your Choice: ";
    cin>>choice;

    switch (choice)
    {
       case 1:cout<<"\nEnter The Number Of Members You Want To Add: ";//add
	      cin>>n;
	      first=new student;
	      cout<<"\nEnter Name: ";
	      gets(first->name);
	      cout<<"Enter Roll: ";
	      cin>>first->roll;
	      first->next=NULL;
	      last=first;
	      for(i=1; i<n; i++)
	      {
		x=new student;
		cout<<"\nEnter Name: ";
		gets(first->name);
		cout<<"Enter Roll: ";
		cin>>first->roll;
		x->next=NULL;
		last->next=x;
		last=x;
	      }
	      break;

       case 3:do//delete
	      {
		if(first!=NULL)
		{
		  cout<<"\nEnter The Roll: ";
		  cin>>val;
		  delet(first, val);
		  display(first);
		}
		else
		{
		  cout<<"\nLIST EMPTY!!!";
		  getch();
		  break;
		}
		cout<<"\n\nDo you want to continue (y/n)?\n";
		cin>>ch;
	      }while(ch=='y'||ch=='Y');
	      break;

       case 4:if (first!=NULL)//display
	      {
		display(first);
	      }
	      else
	      {
		cout<<"\nLIST EMPTY!!!";
	      }
                      getch();
	      break;

     **exit and default**
    }
  }
}

[Duoas]

Er, before anything else, you really need to watch your I/O. Don't mix C++ and C.
Find every getch() (non-standard evil!) and replace it with cin.get(); . Find every gets(s); and replace it with getline( cin, s ); .

Also, I know everyone teaches and tutorials and etc. this, but try not to use cin>>myint; when getting user input. Better to do what the user expects and head-off any input problems by getting a temporary string and converting it:

#include <sstream>
#include <iostream>
using namespace std;

int main() {
  string s;
  int i;

  cout << "Please enter an integer> ";
  getline( cin, s );

  if (!(stringstream( s ) >> i))
    cout << "That wasn't an integer!\n";
  else
    cout << "Thank you\n";

  return EXIT_SUCCESS;
  }

This code is very forgiving about input errors (gets a number whenever it can) and never leaves the input stream messed up.

OK, your linked list. You add a first for the first, then you add an x for every additional. However, you are still modifying first, not x in your input statements. Notice that when you display() the list the last one you entered is displayed properly, but all the others are garbage?

Also, you have memory leaks. What if the user chooses '1' twice? You should initialize first to NULL before anything else in your program. Then check it at the case 1 to see if it is still NULL. If not, you need to either delete the whole list, or add, or give the user a choice, or something.

Well, this has gotten long and I haven't looked at delete yet.

Hope this helps.

[EDIT] Oh yeah, you should either use a string in the student struct for the name or verify that your user doesn't try to give you a name that is too long. Never use gets().

[Duoas]

Eh, sorry for the double post. Here're your problems (yes, more than one) with delet().

Line 24: ptr never becomes NULL unless you are trying to delete the last element.

However, that leaves a list like this
[1] points to [2]
[2] points to [3]
[3] was deleted!
That's a segmentation fault waiting to happen.

I want you to think about this one a bit instead of just giving you the answer. Please do as I ask and get out a piece of paper and pencil and draw yourself a little linked list, full of arrows and everything, and work your way through removing/deleting an element cleanly. Once you understand how to do it on paper you'll be able to tell the computer how to do it with ease.

Hope this helps.

[tracethepath]

i cant stop using getch() and gets()...although i have known that these are non- standard and know the correct ones...since its a school assignment and we have not been taught anything else than what i am using...
sorry for this...

[tracethepath]

and yes i am trying to do what you asked me to do for delet function...it will take some time..so will post my somewhat corrected code after few minutes...hoping you are online till then to help me further..

[tracethepath]

chhanged line 65 and 67 to gets(x->name);
and cin>>x->roll; respectively...
now display function working fine...
still working on delet function...

[tracethepath]

i modified the delet function...now its working fine except when the element to be deleted is first member...

void delet(student*first, int val)
{
  student*ptr,*list,*back;
  if(first->roll==val)
  {
    ptr=first;
    first=first->next;
    delete ptr;
  }
  back=first;
  ptr=ptr->next;
  while(ptr!=NULL)
  {
    if(ptr->roll==val)
    {
      list=ptr;
      back->next=ptr->next;      
      delete list;
     }
     back=ptr;
     ptr=ptr->next;
  }
}

sample input:

MENU (Linked List)

1. CREATE
2. INSERT
3. DELETE
4. DISPLAY
5. EXIT

Enter Your Choice: 1

Enter The Number Of Members You Want To Add: 3

Enter Name: chi
Enter Roll: 13

Enter Name: rou
Enter Roll: 45

Enter Name: csc
Enter Roll: 24

output on deleting:

1. CREATE
2. INSERT
3. DELETE
4. DISPLAY
5. EXIT

Enter Your Choice: 3

Enter The Roll: 13

Current List:

NAME: ■☼■☼╔.ï▬╜▲B╗  ┤
ROLL: 13

NAME: rou
ROLL: 45

NAME: csc
ROLL: 24

Do you want to continue (y/n)?
n

if the member to be deleted is not the first one...then it deletes the element successfully...
i rechecked it but cant find the problem...!!

[Duoas]

Your drawing should have a little arrow pointing from a variable named first in the main() function to the beginning of your list. ;-)

I like your new avatar, btw.

[tracethepath]

changed the return type of delet function from void to student*...
and returned first at the end...
now its working fine...yippie...
i am working on that memory leaks...will prompt user to delete the entire list first through delete option...

thankx a lot...