Linked list sort name and mark according the highest mark

Question

0.00/5 (No votes)

See more:

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:

//...........................................................................

C++

#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

Hatta Schmidt

Updated 18-Dec-12 1:17am

Jochen Arndt

v2

Add a Solution

Comments

Hatta Schmidt 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 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.

4 solutions

Solution 2

C++

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.

Posted 18-Dec-12 18:57pm

Aswin Waiba

Updated 18-Dec-12 19:01pm

v2

Comments

Hatta Schmidt 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 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

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!

Posted 18-Dec-12 21:50pm

nv3

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:

C++

#include <list>

list<node*> personList;

Looking at the code, the Insert is really an Append. Assuming this, the following code performs this:

C++

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:

C++

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:

C++

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...

Posted 19-Dec-12 4:28am

Quirkafleeg

Updated 19-Dec-12 4:30am

v2

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

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

Stefan_Lang · Accepted Answer · 2012-12-18T06:57:00

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.