Sorting linked list in ascending order

Question

I am having trouble sorting a linked list based on their ratings. I am given three tasks, if the list is empty I add the first node, if the passed in node's rating is less than the first node, I move it to the front. If it is greater than the last value, I push it to the back, otherwise I put the node in the its right order.

I am positive functions push(int r, string c), addFirst(int r, string c), and addAtFront(int r, string c) work correctly. I am having trouble implementing the case where the node belongs in between the lowest and highest value.

The sorting function is below:

void SLL::insertInOrder(int r, string c){
SNode *tmp = new SNode(r,c);
if(first == NULL){
    addFirst(tmp->rating,tmp->comments);
}
else if(tmp->rating < first->rating){
    addAtFront(r,c);
}
else if(tmp->rating > last->rating){
    push(r,c);
}
else{
    for(tmp =first; tmp->next != NULL; tmp = tmp->next){
        if(tmp->rating < tmp->next->rating){
            tmp->next = new SNode(r,c);
        }
    }
    }

}

Here is the loop in main as the test:

int r[10] = {9,8,4,5,11,10,3,6,8,2};
    string s[10] = {"really good!","loved it","mediocre",
            "okay, not great","best book ever!", "awesome!",
            "boring","not bad","definitely worth reading", "terrible!"};
    SLL *list = new SLL();
    for (int i = 0; i < 10; i++){
        list->insertInOrder(r[i],s[i]);
        list->printSLL();
    }

My output:

Rating: 9,Comments: really good!

Rating: 8,Comments: loved it
Rating: 9,Comments: really good!

Rating: 4,Comments: mediocre
Rating: 8,Comments: loved it
Rating: 9,Comments: really good!

Rating: 4,Comments: mediocre
Rating: 5,Comments: okay, not great

Rating: 4,Comments: mediocre
Rating: 5,Comments: okay, not great

Rating: 4,Comments: mediocre
Rating: 10,Comments: awesome!

Rating: 3,Comments: boring
Rating: 4,Comments: mediocre
Rating: 10,Comments: awesome!

Rating: 3,Comments: boring
Rating: 6,Comments: not bad

Rating: 3,Comments: boring
Rating: 8,Comments: definitely worth reading

Rating: 2,Comments: terrible!
Rating: 3,Comments: boring
Rating: 8,Comments: definitely worth reading

The output should be:

Rating: 9,Comments: really good! 


Rating: 8,Comments: loved it 
Rating: 9,Comments: really good! 


Rating: 4,Comments: mediocre
Rating: 8,Comments: loved it 
Rating: 9,Comments: really good! 


Rating: 4,Comments: mediocre 
Rating: 5,Comments: okay, not great 
Rating: 8,Comments: loved it 
Rating: 9,Comments: really good! 


Rating: 4,Comments: mediocre 
Rating: 5,Comments: okay, not great
Rating: 8,Comments: loved it 
Rating: 9,Comments: really good! 
Rating: 11,Comments: best book ever! 


Rating: 4,Comments: mediocre 
Rating: 5,Comments: okay, not great
Rating: 8,Comments: loved it 
Rating: 9,Comments: really good! 
Rating: 10,Comments: awesome! 
Rating: 11,Comments: best book ever! 


Rating: 3,Comments: boring 
Rating: 4,Comments: mediocre
Rating: 5,Comments: okay, not great
Rating: 8,Comments: loved it 
Rating: 9,Comments: really good! 
Rating: 10,Comments: awesome!
Rating: 11,Comments: best book ever! 


Rating: 3,Comments: boring 
Rating: 4,Comments: mediocre 
Rating: 5,Comments: okay, not great 
Rating: 6,Comments: not bad 
Rating: 8,Comments: loved it
Rating: 9,Comments: really good! 
Rating: 10,Comments: awesome! 
Rating: 11,Comments: best book ever

I'm having a lot of trouble implementing those intermediate nodes without overwriting the larger values in lists. That last else case is driving me crazy, I've tried many different things.

*I am positive functions push(int r, string c), addFirst(int r, string c), and addAtFront(int r, string c) work correctly.* -- It doesn't work this way here on StackOverflow. Show us these functions. — PaulMcKenzie, Oct 06 '18 at 02:51
The fact that something is fundamentally wrong with the shown approach can be observed by the fact that one expects something like `insertInOrder`() to create just one new node for the linked list. After all, its supposed mission in life is to insert one new node, right? Yet, inexplicably, in this `insertInOrder`(), we have a second `new` in the inner loop which seems to get executable any number of times. Although the overall, high level logic of `insertInOrder`() is not exactly clear, the fact that it can `new` many nodes is fundamentally wrong. Whatever it's trying to do, it can't be right. — Sam Varshavchik, Oct 06 '18 at 02:55
Why do you call the functions to insert at the front or back? All you need is one single loop, and detect where to insert the new node. The only thing "special" is that if the element is smaller than the head node, then you just point it to the head node and make the new node the head. If the loop terminates and no insertion point is found, then you make the new node the tail node by pointing the old tail to the new node. Your function is overly complex calling those specific functions when not necessary. — PaulMcKenzie, Oct 06 '18 at 03:05
The prompt said to use push and insertAtFront for those cases, the else case is to place the node in the correct order for the nodes that do not fit that case — Mike Rawding, Oct 06 '18 at 03:15
@MikeRawding -- Instead of repeating what a "prompt" says, think about what is being done. Does that negate what I stated about how to place an item in a linked list in a sorted fashion? If the item is less than the head node, just make that new item the head node. That accomplishes exactly the same thing, all without calling a function. Similarly with the tail node. If you're not going to post those missing functions, then remove them and just put the new head / tail node in "by hand". — PaulMcKenzie, Oct 06 '18 at 03:25

score 0 · Answer 1 · answered Oct 06 '18 at 03:51

What I see, is that every time you create a new SNode, New_SNode->next is not asigned to the rest of the list. Every time you print the list, previous SNode do not appear.

At the begining declare a SNode *tmp2;

your for loop should be :

 tmp2 = tmp->next;
 tmp->next = new SNode(r,c);
 tmp->next->next->tmp2;

Good luck.

Sorting linked list in ascending order

1 Answers1