2

I built a simple function that, given two arrays aa[5] = {5, 4, 9, -1, 3} and bb[2] = {16, -11}, orders them in a third array cc[7].

#include<stdio.h>
void merge(int *, int *, int *, int, int);

int main(){
    int aa[5] = {5, 4, 9, -1, 3};
    int bb[2] = {16, -11};
    int cc[7];

    merge(aa, bb, cc, 5, 2);

    return 0;
}

void merge(int *aa, int *bb, int *cc, int m, int n){

    int i = 0, j = 0, k = 0;

    while(i < m && j < n){
        if(aa[i] < bb[j])
            cc[k++] = aa[i++];    /*Smallest value should be assigned to cc*/
        else
            cc[k++] = bb[j++];
    }

    while(i < m)              /*Transfer the remaining part of longest array*/
        cc[k++] = aa[i++];


    while(j < n)
        cc[k++] = bb[j++];
}    

The cc array is correctly filled, but the values are not ordered. Instead of the expected cc = {-11, -1, 3, 4, 5, 9, 16} it returns cc = {5, 4, 9, -1, 3, 16, 11}. Like the assignments cc[k++] = aa[i++] and cc[k++] = bb[j++] do not work, somehow, or the logical test if aa[i] < bb[j] goes ignored.

I hypothesized operators priority problems, hence I tested with two different standard, with no differences:

gcc main.c -o main.x -Wall
gcc main.c -o main.x -Wall -std=c89

I checked the code many times, unable to find any relevant error. Any suggestion at this point would be appreciated.

9
  • 2
    @Worice It is supposed that the original arrays are initially ordered. Commented Nov 13, 2017 at 14:42
  • 3
    Your approach appears to assume that the elements of the input arrays are ordered from least to greatest. That is not true of the inputs presented. Commented Nov 13, 2017 at 14:43
  • 2
    That's a very simple program, so this is a great opportunity for you to get familiar with your debugger. Commented Nov 13, 2017 at 14:45
  • @VladfromMoscow, do you mean that such a function cannot work with two unordered arrays? Commented Nov 13, 2017 at 14:46
  • 1
    @Worice Your merge algorithm is guaranteed to fail if either aa or bb are not sorted when it is called. To see this, note that the relative order of the elements within either aa or bb is preserved by the merge. For instance, if element x precedes element y in aa, then x is guaranteed to precede y in cc. There is no mechanism for interchanging them. So, you need to re-think what you're doing. Commented Nov 13, 2017 at 14:57

4 Answers 4

2

You need to think your algorithm through properly. There's no obvious bug in it. The problem is your expectations. One way to make this clear is to think about what would happen if one array was empty. Would the function merge change the order of anything? It will not. In fact, if two elements a and b are from the same array - be it aa or bb - and a comes before b in that array, then a will also come before b in cc.

The function does what you expect on sorted arrays, so make sure they are sorted before. You can use qsort for this.

Other than that, when you use pointers to arrays you do not want to change, use the const qualifier.

void merge(const int *aa, const int *bb, int *cc, int m, int n)
Sign up to request clarification or add additional context in comments.

Comments

2

There's no bug in your implementation (at least I don't see any, imho) The problem is that the merging you have done is not for two sorted arrays (it's for several bunches of sorted numbers). Case you had feed two already sorted arrays you'd have the result sorted correctly.

The merge sorting algorithm begins with splitting the input into two parts of sorted arrays. This is done by switching arrays when you detect the element is not in order (it is not greater to last number) You get the first ordered set to fill an array (the first a elements of initial list which happen to be in order, to put into array A, and the second bunch of elements to put them into array B. This produces two arrays that can be merged (because they are already in order) and this merging makes the result a larger array (this fact is what warrants the algorithm will make larger and larger arrays at each pass and warrants it will finish at some pass. You don't need to operate array by array, as at each pass the list has less and less packs of larger bunch of sorted elements. in your case:

1st pass input (the switching points are where the input 
is not in order, you don't see them, but you switch arrays 
when the next input number is less than the last input one):
  {5}, {4, 9}, {-1, 3}, {16}, {-11}  (See note 2)
after first split:
  {5}, {-1, 3}, {-11}
  {4, 9}, {16}
after first merge result:
  {4, 5, 9}, {-1, 3, 16}, {-11}
after second pass split:
  {4, 5, 9}, {-11}
  {-1, 3, 16}
  result:
  {-1, 3, 4, 5, 9, 16}, {-11}
third pass split:
  {-1, 3, 4, 5, 9, 16}
  {-11}
third pass result:
  {-11, -1, 3, 4, 5, 9, 16}

The algorithm finishes when you don't get two bunches of ordered streams (you don't switch arrays), and you cannot divide further the stream of data.

Your implementation only executes one pass of merge sorting algorithm you need to implement it completely to get sorted output. The algorithm was designed to make it possible to do several passes when input is not feasible to put in arrays (as you do, so it doesn't fully illustrate the thing with arrays). Case you have it read from files, you'll see the idea better.

NOTE

Sort programs for huge amounts of data use merging algorithm for bunchs of data that are quicksort'ed first, so we start with buckets of data that don't fit in an array all together.

NOTE 2

The number 16 after number 3 should have been in the same bunch as previous bunch, making it {-1, 3, 16}, but as they where in different arrays at first, and I have not found any way to put them in a list that splits into this arrangement, I have forced the buckets as if 16 < 3, switching artificially the arrays on splitting the input. This could affect the final result in making an extra pass through the data, but doesn't affect the final result, which is a sorted list of numbers. I have made this on purpose, and it is not a mistake (it has no relevance to explain how the algorithm works) Anyway, the algorithm switches lists (I don't like to use arrays when describing this algoritm, as normally merging algorithms don't operate on arrays, as arrays are random access, while lists must be accessed by some iterator means from beginning to end, which is the requirement of the merging sort algorithm) The same happens to {4, 9}, {16} after the first split, just imagine the result of the comparisons was the one shown, as after the first merge everything is correct.

Comments

1

If your program works fine, you can sort in O(N) by comparison. As it is not possible and mentioned in comments by @Karzes, your program works fine just for the sorted sub arrays. Hence, if you want to implement merge function for the merge sort, you should try your program for these two inputs:

int aa[5] = {-1, 3, 4, 5, 9};
int bb[2] = {-11, 16};

2 Comments

I've never seen a sorting algorithm that does it in O(N). Can you illustrate this? Quicksort is considered the best (imho) at this moment and runs in O(n*log(n)) in the average.
My sentence is "if your program works fine, you can sort in ...", and after that I said "as it is not possible ...".
0

Not the most efficient cause it's bobble sort...

    #include<stdio.h>
    void merge(int *, int *, int *, int, int);
    void sortarray(int array[], int arraySize)
    {
        int c,d,temp;

        for (c = 0 ; c < arraySize-1; c++)
      {
        for (d = 0 ; d < arraySize - c - 1; d++)
        {
          if (array[d] > array[d+1]) /* For decreasing order use < */
          {
            temp       = array[d];
            array[d]   = array[d+1];
            array[d+1] = temp;
          }
        }
      }
    }
    int main(){
        int aa[5] = {5, 4, 9, -1, 3};
        int bb[2] = {16, -11};
        int cc[7];
        int i;
        sortarray(aa,sizeof(aa)/sizeof(aa[0]));

        sortarray(bb,sizeof(bb)/sizeof(bb[0]));

        merge(aa, bb, cc, 5, 2);

         for(i=0;i<sizeof(cc)/sizeof(cc[0]);i++)
         {
            printf("%d,",cc[i]);
         }
        return 0;
    }

    void merge(int *aa, int *bb, int *cc, int m, int n){

        int i = 0, j = 0, k = 0;

        while(i < m && j < n)
        {
            if(aa[i] < bb[j])
                cc[k++] = aa[i++];    /*Smallest value should be assigned to cc*/
            else
                cc[k++] = bb[j++];
        }

        while(i < m)              /*Transfer the remaining part of longest array*/
            cc[k++] = aa[i++];


        while(j < n)
            cc[k++] = bb[j++];
    }  

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.