Merge sort algorithm in C not working as expected

I am trying to implement the merge sort algorithm in C. I understand how the algorithm is supposed to work however I am encountering some difficulties with the implementation.

I understand that there are hundreds of examples and source code for it's implementation but I was hoping someone could help me understand why mine is not working correctly.

My code is below and after the code I explain what I have tried so far.

#include <stdio.h>

void merge(int a[], int L[], int R[],int nL, int nR) //nL and nR are the lengths of L[] and R[]
{
   int i = 0 , j = 0, k = 0;

    while(i<nL && j <nR)
    {
        if(L[i] <= R[j]){
            a[k] = L[i];
            i++;
        }
        else{
            a[k] = R[j];
            j++;
        }

        k++;
     }

     while(i < nL){
        a[k] = L[i];
        i++;
        k++;
     }

     while(j < nR) {
        a[k] = R[j];
        j++;
        k++;
     }
}   

void mergesort(int a[],int n)    //n is the length of a[]
{
   if(n < 2) return;     //BASE CASE
   int mid = n / 2;

   int left[mid];
   int right[n-mid];

   for(int i = 0; i < mid; i++)
   {
        left[i] = a[i];
   }

   for(int i = mid; i < n-1; i++)
   {
        right[i-mid] = a[i];
   }

   int nL = sizeof(left) / sizeof(left[0]);
   int nR = sizeof(right) / sizeof(right[0]);

   mergesort(left, nL);
   mergesort(right, nR);
   merge(a,left,right,nL,nR); 
}    


int main(void)
{
    printf("Initial:\n");
    printf("3 4 1 6\n");

    int numbers[4] = {3,4,1,6};
    int n = sizeof(numbers) / sizeof(int);

    mergesort(numbers,n);
    printf("Sorted:\n");
    for(int i =0 ; i < 4; i++)
    {
        printf("%d ", numbers[i]);
    }

    return 0;
}

As it is and with the unsorted array [3,4,1,6] the output is 0 0 1 3. Clearly the 1 and 3 are in the right order relative to each other but the two zeros at the beginning are clearly wrong. At first it seemed to me that I was inserting 4 and 6 to the right and out of bounds of the array.

I used some print statements to try and debug but I haven't been able to figure out what was going on. I even tried to follow my code with gdb but I still could not sort it.

Does any one have any ideas of what might be happening?

Solution

A more nearly idiomatic way of writing the merge() code would be:

void merge(int a[], int L[], int R[],int nL, int nR)
{
    int i = 0, j = 0, k = 0;

    while (i < nL && j < nR)
    {
        if (L[i] <= R[j])
            a[k++] = L[i++];
        else
            a[k++] = R[j++];
    }
    while (i < nL)
        a[k++] = L[i++];  
    while (j < nR)
        a[k++] = R[j++];
}

That's about half the number of lines of your code, and within broad limits, the less code there is to read, the better. There are those who insist on having braces after each loop or conditional. I don't think that's necessary (or particularly helpful), but if that's the style you like, you can use it.

Your mergesort() code is less flabby, but could be changed to:

void mergesort(int a[],int n)    //n is the length of a[]
{
    if (n < 2)
        return;     //BASE CASE
    int mid = n / 2;
    int left[mid];
    int right[n-mid];

    for (int i = 0; i < mid; i++)
        left[i] = a[i];

    for (int i = mid; i < n; i++)
        right[i-mid] = a[i];

    mergesort(left, mid);
    mergesort(right, n - mid);
    merge(a, left, right, mid, n - mid); 
}

This includes the fix for your main problem — the loop loading the right array was leaving the last element uncopied.

With a debugging function such as:

void dump_array(const char *tag, int n, int *a)
{
    printf("%s:%d:", tag, n);
    for (int i = 0; i < n; i++)
        printf(" %3d", a[i]);
    putchar('\n');
}

You can do a lot of effective debugging with:

void mergesort(int a[],int n)
{
    if (n < 2)
        return;
    int mid = n / 2;
    int left[mid];
    int right[n-mid];

    dump_array("-->>mergesort()", n, a);

    for (int i = 0; i < mid; i++)
        left[i] = a[i];
    dump_array("left", mid, left);

    for (int i = mid; i < n; i++)
        right[i-mid] = a[i];
    dump_array("right", n - mid, right);

    mergesort(left, mid);
    dump_array("merged-L", mid, left);
    mergesort(right, n - mid);
    dump_array("merged-R", n - mid, right);
    merge(a, left, right, mid, n - mid);
    dump_array("<<--mergesort()", n, a);
}

In your code, the output with the tag right would show 0 or semi-random data for the last element, rather than what you're expecting. This would be a hint as to where the trouble is. Keep the dump_array() function around; it is a useful creature to have. It's a simple-minded version; you can invent more complex versions which outputs a newline at intermediate positions for long arrays, for example.