views:

109

answers:

2

How come void del_begin() crashes when there's only one node left (I have other functions to add nodes)?

 #include <iostream>
 using namesspace std;

 node *start_ptr = NULL;
 node *current;
 int option = 0;
 void del_end()
 {
     node *temp, *temp2;
     temp = start_ptr;
     if (start_ptr == NULL)
         cout << "There are no nodes" << endl;
     else
     {
         while (temp->nxt != NULL)
         {
            temp2 = temp;
            temp = temp->nxt;
         }
         delete temp;
         temp2->nxt = NULL;
     }
 }
 void display()
 {
     node *temp;
     temp = start_ptr;
     cout << endl;
     if (temp == NULL)
         cout << "There are no nodes to display" << endl;
     else
     {
         while(temp != NULL)
         {
        cout << temp->name << ", " << temp->profession << ", " << temp->age;
        if (temp == current)
            cout << "***";
        cout << endl;
        temp = temp->nxt;
        }
        cout << endl;
     }
 }

 int main()
 {
     start_ptr = NULL;
     int option;
      do
     {
         display();
         cout << "0 for EXIT" << endl;
         cout << "1 to ADD TO END" << endl;
         cout << "2 to ADD TO BEGINNING" << endl;
         cout << "3 to DELETE LAST" << endl;
         cout << "4 to DELETE BEGINNING" << endl;
         cout << ">>";
         cin >> option;
         switch (option)
         {
         case 1 : add_end(); break;
         case 2 : add_begin(); break;
         case 3 : del_end(); break;
         case 4 : del_begin(); break;
         }
     }
     while (option != 0);
     return 0;
 }
+1  A: 

You didn't show us the code for del_begin(), but your del_end() has a bug in the case you're mentioning (single node list).

If you have only one node, your while loop will never execute, and temp2 will be uninitialized when you get to the line:

 temp2->nxt = NULL;

Crash!

Carl Norum
How do i fix it?
danutenshu
@danutenshu: You fix it by using a library list class instead. Is there some reason you're forced to roll your own?
Anon.
I'm being assigned to make my own link list. Could you explain more? I'm not seeing the
danutenshu
not only that - if temp is one and only element and it is deleted, he tries to set NULL value on (deleted element)->nxt.@danutenshu just initialize temp2, in your case adding the line `temp2 = start_ptr` and changing the order of `delete temp` and `temp2->nxt = NULL` should do the job. However, your code is far from being pretty ;/
doc
what's considered pretty?
danutenshu
PS. Also set start_ptr to NULL, when you deleted last element.
doc
Can somebody give me live help on aim? I edited it and now it's even worse
danutenshu
@danutenshu, why don't you ask your professor or a classmate? They are likely to be more sympathetic.
Carl Norum
A: 

If you have only one node, your while loop will never execute, and temp2 will be uninitialized

start_ptr & current are not reset appropriately on deletion.

This is not thread safe (in all sorts of ways) but for example you delete the next item before removing it from the list.

Greg Domjan