CODEWITHSUNDEEP
csortingmergelinked-list
Data
Data

Reputation: 769

Nodes lost in linked list merge sort implementation in C

This implementation is used to order strings in ascending alphabetical order. I've included the functions responsible for the dividing of nodes. head is the head of the linked list.

I believe the mergeSort function is working as intended to divide the list up. See the following ouput:

Head: lady apple stone red queen
fast:
slow: stone
front: lady apple stone
back: red queen

Head: lady apple stone
fast:
slow: apple
front: lady apple
back: stone

Head: lady apple
fast: 
slow: lady
front: lady
back: apple

Head: red queen
fast:
slow: red
front: red
back: queen

Which clearly shows the initial list, lady apple stone red queen being divided into individual nodes, all accounted for.

The problems come when nodes are compared and merged into new lists. For example: Initially, lady is compared to apple. They're merged into the list: apple, lady. Then this list should be merged with stone. However, first, lady is compared with stone, instead of apple being compared to stone. This then generates the list: lady, stone (apple being left behind and not used in a comparison). What should happen, is that apple is compared to stone, then lady is compared to stone, and then the entries are sorted and merged accordingly into: apple, lady, stone.

The actual final output is: lady, red, stone. Clearly, apple and queen have been lost somewhere. I'm not sure what the offending code is. 

void mergeSort(Node *head) {

    Node *front = NULL, *back = NULL, *fast, *slow;

    if(head == NULL || head->next == NULL)
        return;

    ...

    mergeSort(front);
    mergeSort(back);

    head = mergeLists(front, back);

}

Upvotes: 1

Views: 304

Answers (3)

wildplasser
wildplasser

Reputation: 44240

The slow/fast method (Floyds algoritm) is not needed. It is also counter -effective: the sorting will always take N log(N) operations, even on a sorted list.

You can take advantage of the existing order by only cutting out the out-of-order elements, and keeping the rest. Then (recursively) sort the unordered part, and merge the two parts.

struct list {
        struct list *next;
        char* data;
        };

#define COMPARE(a,b) strcmp((a),(b))

struct list *splitmerge(struct list *org)
{
struct list *rest ,*this, **pp;

if(!org|| !org->next) return org;

        /* Split off out-of-order nodes*/
rest = NULL;
for (this = org; this;  ){
        struct list* next;
        if(!this->next)break;

        next = this->next;
        if(COMPARE( this->data,  next->data) <= 0) {
                this= next;continue;
                }
                /* Cut next out of the chain */
        this->next = next->next;
        next->next = rest;
        rest = next;
        }

        /* At this point, org is guaranteed to be sorted
        ** rest is not.
         */


        /* Order the trash */
rest = splitmerge(rest);


        /* Merge with the trash */
this=NULL;
for(pp = &this; org && rest; pp = &(*pp)->next ) {
        if(COMPARE( org->data, rest->data) <= 0) {
                *pp = org; org=org->next;
                }
        else    {
                *pp = rest; rest=rest->next;
                }
        }
*pp = (org) ? org : rest;

return this;
}

Upvotes: 0

abdullah
abdullah

Reputation: 662

Your code is almost perfect. You are merging two lists and returning it to head. But you are not returning that exact head to previous mergeSort calls. 

Node* mergeSort(Node *head) {

    Node *front = NULL, *back = NULL, *fast, *slow;

    if(head == NULL || head->next == NULL)
        return head; // return head
    ...

    front = mergeSort(front); // save it to front
    back = mergeSort(back); // save it to back

    head = mergeLists(front, back); // save it to head
    return head; // return head
}

In your main function where you are calling mergeSort, use head = mergeSort(head). The code should work now.

Upvotes: 2

klutt
klutt

Reputation: 31296

An easier (and probably more efficient, especially for large lists) method is to first convert the linked list to an array. Consider this code:

char ** arr;
int size=getLinkedListSize(head);
arr = malloc(sizeof(*arr)*size);
linkedList2Array(head, arr);
mergeSort(arr, size);
array2LinkedList(arr, size, head);

Note that I'm using the type char **. The point (hehe) is to point at the data in the linked list. If the data there is more complex that simple chars or ints you avoid unnecessary copying of data.

This is much easier to write, and even though it has some boilerplate it does not affect anything. The functions (which you have to write) getLinkedListSize, linkedList2Array and array2LinkedList can all have a complexity of O(n) while mergeSort will be O(n*log n) no matter what you do. Also, take into account that this method is MUCH more friendly to the cache.

The only downside of this is that it requires a little more memory, but it is not by much. The array will be at maximum the same size as the list.

The three functions I mentioned are trivial to write. Here are two of them:

int getLinkedListSize(Node * head)
{
    int ret=0;
    while(head) {
        ret++;
        head=head->next;
    }
    return ret;
}

void linkedList2Array(Node * head, char ** arr)
{
    int index=0;
    while(head) {
        arr[index++]=&head->data;
        head=head->next;
    }
}

Upvotes: 1

Related Questions