I'm coding a mergesort that splits into 3, i have no idea why it does not work

Question

This is all i have in my code, it should display the numbers arranged. The code is supposed to split the array of numbers into 3 instead of into 2 like the normal mergesort would. I have checked every part of it and it and I cannot find why it does not work

#include <iostream>

using namespace std;

void merge(int*,int*,int,int,int,int);

//mergesort
void mergesort(int *a, int*b, int low, int high)
{

    if(high - low <2)
      return;

    int pivot1 = low+((high-low)/3);
    int pivot2 = low+2*((high-low)/3)+1;
    mergesort(a,b,low,pivot1);
    mergesort(a,b,pivot1,pivot2);
    mergesort(a,b,pivot2,high);
    merge(a,b,low,pivot1,pivot2,high);
}

//merge
void merge(int *a, int *b, int low, int pivot1, int pivot2, int high)
{
    int h,i,j,l;
    h=low;
    i=low;
    j=pivot1;
    l=pivot2;

    while((h<pivot1)&&(j<pivot2)&&(l<high))
    {
        if(a[h]<a[j])
        {
          if(a[h] < a[l]){
            b[i] = a[h];
            i++;
            h++;
          }
          else{
            b[i] = a[l];
            i++;
            l++;
          }
       }
       else{
         if(a[j] < a[l]){
          b[i] = a[j];
          i++;
          j++;
         }
         else{
           b[i] = a[l];
           i++;
           l++;
         }
       }
    }

    while((h < pivot1) && (j < pivot2)){
      if(a[h] < a[j]){
    b[i] = a[h];
    i++;
    h++;
      }
      else{
    b[i] = a[j];
    i++;
    j++;
      }
    }

    while((j < pivot2) && (l < high)){
      if(a[j] < a[l]){
    b[i] = a[j];
    i++;
    j++;
      }
      else{
    b[i] = a[l];
    i++;
    l++;
      }
    }

    while((h < pivot1) && (l < high)){
      if(a[h] < a[l]){
    b[i] = a[h];
    i++;
    h++;
      }
      else{
    b[i] = a[l];
    i++;
    l++;
      }
    }
    while(h < pivot1){
      b[i] = a[h];
      i++;
      h++;
    }
    while(j < pivot2){
      b[i] = a[j];
      i++;
      j++;
    }
    while(l < high){
      b[i] = a[l];
      i++;
      l++;
    }
}

int main()
{
    int a[] = {12,10,43,23,-78,45,123,56,98,41,90,24};
    int num;

    num = sizeof(a)/sizeof(int);

    int b[num];

    mergesort(a,b,0,num-1);

    for(int i=0; i<num; i++)
        cout<<b[i]<<" ";
    cout<<endl;
}

it should display the sorted version of the array

Please update your question: your code does not compile. I would also suggest to reformat it, i.e. remove unnecessary empty line and ensure correct indentation. — Stefan Becker, Sep 19 '19 at 05:44
The call from main to mergesort should probably be `mergesort(a,b,0,num);` (not num-1). Then pivot2 = low+2*((high-low)/3); — rcgldr, Sep 19 '19 at 17:33

Stefan Becker · Answer 1 · 2019-09-22T12:06:10.583

Your code does not compile. To pass with g++ -Wall -Werror I needed to add the following lines to the start of the file:

#include <iostream>

using namespace std;

Your code only reads from array a and writes to array b. Nowhere the "sorted" contents from b are copied back to a.

My guess your intention is that that should happen in merge(), i.e. by adding to the end of the function:

for(int i=low; i<high; i++)
    a[i] = b[i];

Furthermore there is an off-by-one error in main(): mergesort() expects high to be the total number of items to be sorted and not the index of the highest item in the array:

mergesort(a,b,0,num);

With those changes the code compiles and prints out the correct result

-78 10 12 23 24 41 43 45 56 90 98 123

EDIT

Passing the array b around is also unnecessary as it is in reality only needed to perform the merge. Hence the code can be updated to read:

void merge(int*,int,int,int,int);

//mergesort
void mergesort(int *a, int low, int high)
{
    ...
    mergesort(a,low,pivot1);
    mergesort(a,pivot1,pivot2);
    mergesort(a,pivot2,high);
    merge(a,low,pivot1,pivot2,high);
}

//merge
void merge(int *a, int low, int pivot1, int pivot2, int high)
{
    int b[high];
    ...
}

int main()
{
    int a[] = {12,10,43,23,-78,45,123,56,98,41,90,24};
    int num = sizeof(a) / sizeof(*a);

    mergesort(a,0,num);

    for(int i=0; i<num; i++)
        cout<<a[i]<<" ";
    cout<<endl;
}

EDIT 2

The memory consumption of merge() can be reduced, because in each invocation it only sorts the slice a[low..high-1]. Hence the code can be updated to read:

//merge
void merge(int *a, int low, int pivot1, int pivot2, int high)
{
    int b[high - low]; // slice a[low..high - 1] to be sorted
    int i = 0;         // running index in b[]
    ...
    // replace slice a[low.. high - 1] with sorted result
    for (i = low; i < high; i++)
       a[i] = b[i - low];
}

BONUS

Complete code simplified and more readable:

#include <iostream>

using namespace std;

static void merge(int *a,
                  const unsigned int low,    const unsigned int pivot1,
                  const unsigned int pivot2, const unsigned int high);

// mergesort (recursive)
static void mergesort(int *a, const unsigned int low, const unsigned int high)
{
    const unsigned int slice = high - low;

    // terminate recursion
    if (slice < 2)
        return;
    if (slice < 3) {
        // can't recurse for slice of length 2; just sort using 2 partitions
        merge(a, low, low, low + 1, high);
        return;
    }

    const unsigned int third  = slice / 3;
    const unsigned int pivot1 = low  + third;
    const unsigned int pivot2 = high - third;

    // recurse
    mergesort(a, low,    pivot1);
    mergesort(a, pivot1, pivot2);
    mergesort(a, pivot2, high);

    // merge
    merge(a, low, pivot1, pivot2, high);
}

// merge
static void merge(int *a,
                  const unsigned int low,    const unsigned int pivot1,
                  const unsigned int pivot2, const unsigned int high)
{
    int sorted[high - low];           // slice a[low..high - 1] to be sorted
    int *store = sorted;              // running pointer into sorted[]
    unsigned int partition1 = low;    // running index a[low    .. pivot1 - 1]
    unsigned int partition2 = pivot1; // running index a[pivot1 .. pivot2 - 1]
    unsigned int partition3 = pivot2; // running index a[pivot2 .. high   - 1]

    while ((partition1 < pivot1) &&
           (partition2 < pivot2) &&
           (partition3 < high)) {
        if (a[partition1] < a[partition2])
            *store++ = a[partition1] < a[partition3] ? a[partition1++] : a[partition3++];
        else
            *store++ = a[partition2] < a[partition3] ? a[partition2++] : a[partition3++];
    }

    while ((partition1 < pivot1) &&
           (partition2 < pivot2)) {
        *store++ = a[partition1] < a[partition2] ? a[partition1++] : a[partition2++];
    }

    while ((partition2 < pivot2) &&
           (partition3 < high)) {
        *store++ = a[partition2] < a[partition3] ? a[partition2++] : a[partition3++];
    }

    while ((partition1 < pivot1) &&
           (partition3 < high)) {
        *store++ = a[partition1] < a[partition3] ? a[partition1++] : a[partition3++];
    }

    while (partition1 < pivot1)
        *store++ = a[partition1++];

    while (partition2 < pivot2)
        *store++ = a[partition2++];

    while (partition3 < high)
        *store++ = a[partition3++];

    // replace slice a[low.. high - 1] with sorted result
    for (unsigned int i = low; i < high; i++)
        a[i] = sorted[i - low];
}

int main()
{
    int a[] = {12,10,43,23,-78,45,123,56,98,41,90,24};
    // int a[] = {-78,10,12,23,24,41,43,45,56,90,98,123};
    // int a[] = { 4, 3, 2, 1, 0};
    // int a[] = { 3, 2, 1, 0};
    // int a[] = { 2, 1, 0};
    // int a[] = { 1, 0};
    // int a[] = { 0};
    const unsigned int num = sizeof(a) / sizeof(*a);

    mergesort(a, 0, num);

    for (unsigned int i = 0; i < num; i++)
        cout << a[i] <<" ";
    cout << endl;
}

BONUS 2

Reimplemented merge() function:

static void merge(int *a,
                  const unsigned int low,    const unsigned int pivot1,
                  const unsigned int pivot2, const unsigned int high)
{

    unsigned int slice = high - low;
    int sorted[slice];                // slice a[low..high - 1] to be sorted
    unsigned int store      = 0;      // running pointer into sorted[]
    unsigned int partition1 = low;    // running index a[low    .. pivot1 - 1]
    unsigned int partition2 = pivot1; // running index a[pivot1 .. pivot2 - 1]
    unsigned int partition3 = pivot2; // running index a[pivot2 .. high   - 1]

    while (store < slice) {
        int next;

        if (partition1 < pivot1) {
            // partition1 is not exhausted yet
            next = a[partition1];

            if (partition2 < pivot2) {

                // partitions 1 & 2 are not exhausted yet
                if (partition3 < high) {

                    // all partitions are not exhausted yet
                    if (next < a[partition2]) {
                        if (a[partition2] < a[partition3])
                            partition1++;           // p1 <  p2 <  p3
                        else if (next < a[partition3])
                            partition1++;           // p1 <  (p3 || p2)
                        else
                            next = a[partition3++]; // p3 <= p1 <  p2

                    } else if (a[partition2] < a[partition3])
                          next = a[partition2++];   // p2 <  p3 <= p1
                    else
                        next = a[partition3++];     // p3 <= p2 <= p1

                } else if (next < a[partition2])
                    partition1++;                   // p1 <  p2 [p3]
                else
                    next = a[partition2++];         // p2 <= p1 [p3]

            } else if (partition3 < high) {
                // partition2 is exhausted, compare p1 & p3 only
                if (next < a[partition3])
                    partition1++;                   // p1 <  p3 [p2]
                else
                    next = a[partition3++];         // p3 <= p1 [p2]

            } else
                // partitions 2 & 3 are exhausted, copy from p1
                partition1++;                       // p1 [p2, p3]

        } else if (partition2 < pivot2) {
            // partition1 is exhausted, compare p2 & p3 only
            next = a[partition2];

            if (partition3 < high) {

                // partitions 2 & 3 are not exhausted
                if (next < a[partition3])
                    partition2++;                   // p2 <  p3 [p1]
                else
                    next = a[partition3++];         // p3 <= p2 [p1]

            } else
                // partitions 1 & 3 are exhausted, copy from p2
                partition2++;                       // p2 [p1, p3]

        } else {
            // partitions 1 & 2 are exhausted, copy from p3
            next = a[partition3++];                 // p3 [p1, p2]
        }

        // store smallest entry
        sorted[store++] = next;
    }

    // replace slice a[low.. high - 1] with sorted result
    for (unsigned int i = low; i < high; i++)
        a[i] = sorted[i - low];
}

The call from main to mergesort should probably be mergesort(a,b,0,num); (not num-1). Then pivot2 = low+2*((high-low)/3); . — rcgldr, Sep 19 '19 at 17:34
@rcgldr the code goes into an endless loop if you make that change. — Stefan Becker, Sep 20 '19 at 06:44
There's a problem when (high - low == 2). Try using `pivot2 = high - third;` — rcgldr, Sep 20 '19 at 06:57
@rcgldr that is not enough. `mergesort()` must not recurse for `high - low == 2`. — Stefan Becker, Sep 22 '19 at 12:08
@rcgldr no it is not recursive for `slice == 2`: it's calling `merge()` not `mergesort()`. Agreed performing a simple swap is simpler. — Stefan Becker, Sep 22 '19 at 15:41
You're correct, call merge or swap in place for slice == 2. I have a [4-way merge sort](https://stackoverflow.com/questions/34844613/34845789#34845789) (the last part of my answer), but it's a bottom up merge sort (slightly faster), and uses swaps for slice == 2 or slice == 3. That way the initial merge is guaranteed at least 1 element in all 4 runs when it starts (it still has to drop down to 3 way, then 2 way, then 1 way copy). — rcgldr, Sep 22 '19 at 20:25
The other option is a combination of my prior (and now deleted) comments: | third = (high + 1 - low) / 3; | pivot1 = low + third; | pivot2 = high - third; |. Then the recursive calls would be mergesort (a, low, pivot1) (slice = 1), mergesort(a, pivot1, pivot2) (slice = 0), mergesort(a, pivot2, high) (slice = 1). This will stop infinite recursion, but merge would be dealing with an empty slice, which your version already does. — rcgldr, Sep 23 '19 at 02:09

I'm coding a mergesort that splits into 3, i have no idea why it does not work

1 Answers1

EDIT

EDIT 2

BONUS

BONUS 2