Click here to Skip to main content
Rate this: bad
good
Please Sign up or sign in to vote.
See more: C++
I have create 1 program which is to insert name and mark than sort it with highest mark.When i run it everything find such as when i 1-create data and 2-display.But,when i choose 3-Sort, it will loop forever until infinity.Hope someone can help me.Very appreciate that.Thanks,below the code:
 
//...........................................................................
#include <iostream>
#include <cstdlib>
#include <string.h>
#include <iomanip>
#include <windows.h>
using namespace std;
struct node
    {
        int mark;
        char name[10];
        struct node *next;
    } *person=NULL,*head=NULL,*temp=NULL,*curr=NULL,*head1=NULL,*current=NULL,*o=NULL,*next=NULL;
 
void SortList();
void Insert();
 
void Menu()
{
    cout<<"1.Insert data."<<endl;
    cout<<"2.Display data."<<endl;
    cout<<"3.Sort data based on highest mark."<<endl;
    cout<<"4.Exit."<<endl;
    cout<<"Enter Your choice: ";
}
 
void Insert()
{
    person =new node;
    char name[10];
    cout<<endl;
    cout<<"Enter Your name: ";
    cin>>person->name;
    cout<<"Enter Your mark: ";
    cin>>person->mark;
    cout<<endl;
 
    if (head == NULL)
    {
        head = person;
        person->next = NULL;
    }
    else
    {
        person->next = head;
        head = person;
    }
}
 
void Print()
{
    temp = head;
    int i = 1;
 
    cout<<"-----LIST CONTENTS-----"<<endl<<endl;
    cout<<"No"<<setw(10)<<"Name"<<setw(10)<<"Marks"<<endl;
 
    if (temp == NULL)
   cout<<"List is empty!!";
    else
    {
        while (temp != NULL)
        {
            cout<<i<<setw(10)<<temp->name<<setw(10)<<temp->mark<<endl;
            temp = temp->next;
            i++;
        }
 
    }
    cout<<endl;
}
 
void SortedInsert()
{
    temp = head;
    person = o;
    if (head == NULL)
    {
        head = person;
        person->next = NULL;
    }
    else
    {
        if (person->mark > temp->mark)
        {
             person->next = temp;
             head = person;
 
        }
        else
        {
            while ((temp->next != NULL) && (temp->next->mark > person->mark))
            {
                temp = temp-> next;
            }
 
                person->next=temp->next;
                temp->next = person;
 
            }
        }
    }
 

void SortList()
{
 
    current = head;
    if (current == NULL)
    cout<<"List empty"<<endl;
    else
    {
        while (current != NULL)
        {
            next = current->next;
            SortedInsert();
            current=next;
        }
 
    }
 

    cout<<"Sorted list: "<<endl;
 
}
int main (int argc, const char * argv[])
{
 
    int choice;
    cout<<"===This Program to insert name and mark==="<<endl;
    cout<<endl;
    do
    {
        Menu();
        cin>>choice;
        switch (choice)
        {
            case 1:
                Insert();
                break;
            case 2:
                Print();
                break;
            case 3:
                SortList();
                Print();
                break;
            default:
               cout<<"\nWRONG CHOICE";
        }
    } while (choice != 0);
 
    return 0;
}
Posted 18-Dec-12 1:13am
Edited 18-Dec-12 1:17am
v2
Comments
Hatta Schmidt at 18-Dec-12 7:25am
   
which part u edit Jochen Arndt??
this program nothing happen when i choose number 3-sort data based on high mark.
please someone help me!!Thanks
Jochen Arndt at 19-Dec-12 4:19am
   
Sorry for my late reply.
I inserted formatting tags around the code only. No visible text or code has been changed. The formatted code can be read much better than before. If you click the above v2 link, you will see a short comment from me about my changings and you can compare the revisions.
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 1

I wonder how this doesn't crash: You initialize the global variable o to NULL and never change it. In SortedInsert() you assign that to person, and then you dereference person (which at that point is a NULL pointer).
 
On a side note, the function SortList() uses and modifies the global variables current and next, you pass no variables to SortedInsert, and SortedInsert accesses neither of these two variables! How can these functions possibly interact?
 
Do yourself a favor and remove all those global variables! Use local variables instead and function arguments. it will make the flow of control much clearer, and you'll get a better view of which variable accesses what.
  Permalink  
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 2

void SortedInsert()
{
    temp = head;
    person = o; // where is o assigned a valid value
                // seems like you are using an uninitialized variable
    if (head == NULL)
    {
        head = person;
        person->next = NULL;
    }
    else
    {
        if (person->mark > temp->mark)
        {
             person->next = temp;
             head = person;
 
        }
        else
        {
            while ((temp->next != NULL) && (temp->next->mark > person->mark))
            {
                temp = temp-> next;
            }
 
                person->next=temp->next;
                temp->next = person;
 
            }
        }
    }
But seriously I am amazed that your program does not crash when you call the SortList() function.
You should be getting a seg fault.
  Permalink  
v2
Comments
Hatta Schmidt at 19-Dec-12 2:18am
   
Thanks..but i still don't get the correct one.please help me,what kind of code that i need to replace it.
Aswin Waiba at 20-Dec-12 9:44am
   
Sorry for the late reply. You could try simplyfying your code by copying data instead of rearranging the links, which can get a bit tricky.
 
void SortList()
{
if( NULL == head || NULL ==head )
cout<<"Linked list is empty or contains only one element"<<endl;
node *counter1, *counter2;

counter2 = head->next;
for(counter1 = head; counter1!=NULL ; counter1=counter2->next)
{
for(counter2 =counter1->next; counter2!=NULL; counter2=counter2->next)
{
compare(counter1,counter2);
}
}
}
 
void compare(node *counter1,node *counter2)
{
char tempName[10];
int tempMark;
if(counter2->mark > counter1->mark)
{
tempMark = counter2->mark;
strcpy(tempName, counter2->Name)

counter2->mark = counter1->mark;
strcpy(counter2->mark,counter1->mark);

counter1->mark = tempMark;
strcpy(counter1->Name, tempName);
}
}
 
Just another suggestion. Try using local variables. It makes the code much simpler to read and understand and stops the problem of phantom amends to variables. I hope this helps
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 3

Looks to me that you copied someone else's code, but didn't mixed it up with your own thoughts afterwards. To fix it, you should first devise a clear strategy what SortList and SortedInsert should do; somethings along these lines:
 
SortList: Will remove a single element from the list pointed to by head and enter that element into a new list pointed to by head1 (you already declared it, but never used it so far). After the entire list head has been processed, list head1 now contains the sorted list. Finally it will assign head1 to head.
 
SortedInsert: Will look at the current element selected (and removed) by SortList and it to list head1 in a place, so that list head1 is always in sorted order.
 
Then modify SortList and SortedInsert accordingly.
 
Hint: Follow Stefan's advice and get rid of all those global variables. For exanple, SortList could pass the "current" element that is to be inserted by a parameter to SertedInsert instead via a global pointer.
 
After you have done that and if you then still have problems with your code, show us what you have done by pasting your new code via "Improve Question".
 
Good Luck!
  Permalink  
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 4

There is a bit of a mis-mash here - a mix of C (char array instead of std::string) and C++ style programming (cout). For such a program, I doubt the performance requires the use of char arrays over std::string.
 
So... C++ STL already contains a doubly linked list - list<>.
 
If your intention is to create your own linked list, rather than use an in-built one, the following probably has no use. However, the following "simplifies" (this is C++!) what you trying to do.
 
Instead of creating a structure that contains a pointer to the next node, you could remove that pointer member and include the following:
 
#include <list>

list<node*> personList;
 
Looking at the code, the Insert is really an Append. Assuming this, the following code performs this:
 
void Insert()
{
    node* person = new node();
    char name[10];
    cout<<endl;
    cout<<"Enter Your name: ";
    cin>>person->name;
    cout<<"Enter Your mark: ";
    cin>>person->mark;
    cout<<endl;
 
    personList.push_back(person);
}
 
Sorting is far easier to work with:
 
bool comparePeople(node* first, node* second)
{
    return (first->mark > second->mark);
}
 
void SortList()
{
    if (personList.empty())
        cout<<"List empty"<<endl;
    else
    {
        personList.sort(comparePeople);
        cout<<"Sorted list: "<<endl;
    }
}
 
To output, you would do the following, instead of using the while loop:
 
int i = 1;
for (list<node*>::iterator it = personList.begin(); it != personList.end(); ++it)
{
    cout<<i<<setw(10)<<(*it)->name<<setw(10)<<(*it)->mark<<endl;
    ++i;
}
 
(if you like extra functors, or can use lambda expressions (C++0x support required), you should use for_each instead)
 
Finally, which is what you have to work out yourself, there are 2 errors that need to be worked upon:
1) You can choose 4 to exit, but the application never exits
2) You must delete all objects you create (person in Insert). This should be before the "return 0" in main. I would suggest using for_each for this...
  Permalink  
v2

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

  Print Answers RSS
0 Sergey Alexandrovich Kryukov 511
1 OriginalGriff 344
2 DamithSL 235
3 Maciej Los 140
4 Bruno Sprecher 130
0 DamithSL 185
1 OriginalGriff 105
2 Peter Leow 60
3 Maciej Los 50
4 abhijeetgupta1988 45


Advertise | Privacy | Mobile
Web02 | 2.8.141030.1 | Last Updated 19 Dec 2012
Copyright © CodeProject, 1999-2014
All Rights Reserved. Terms of Service
Layout: fixed | fluid

CodeProject, 503-250 Ferrand Drive Toronto Ontario, M3C 3G8 Canada +1 416-849-8900 x 100