Search code examples
ccounting-sort

Why this counting sort return input instead of sorted table?


I'm writing counting sort in C. N is the number of elements in table which is to be sorted, k is max value that any of this element can be. However, this code, leaves me with the same table as the input. What's wrong?

void countingSort(int *tab, int n, int k) {
    int *counters = (int *)malloc(k * sizeof(int));
    int *result = (int *)malloc(n * sizeof(int*));
    for (int i = 0; i < k; i++) {
        counters[i] = 0;
    }
    for (int i = 0; i < n; i++) {
        counters[tab[i]]++;
    }
    int j = 0;
    for (int i = 0; i < k; i++) {
        int tmp = counters[i];
        while (tmp--) {
            result[j] = i;
            j++;
        }
    }
    tab = result;
}


Solution

  • There are some problems in your code:

    • int *result = (int *)malloc(n * sizeof(int*)); uses an incorrect size. The array element type is int, not int*. You should write:

      int *result = (int *)malloc(n * sizeof(int));
      

      or better:

      int *result = (int *)malloc(n * sizeof(*result));
      

      note also that the cast is useless in C, unlike C++ where it is mandatory:

      int *result = malloc(n * sizeof(*result));
      
    • you could avoid the extra initializing loop by using calloc():

      int *counters = calloc(n, sizeof(*counters));
      
    • a major problem: the result array is never returned to the caller: tab = result; just modifies the argument value, not the caller's variable. You should instead use the tab array to store the results directly.

    • you do not free the arrays, causing memory leaks.

    • you do not test for allocation success, causing undefined behavior if memory is not available. You should return an error status indicating this potential problem.

    Here is a corrected version:

    // assuming all entries in tab are > 0 and < k
    int countingSort(int *tab, int n, int k) {
        int *counters = calloc(k, sizeof(*counters));
        if (counters == NULL)
            return -1;
        for (int i = 0; i < n; i++) {
            counters[tab[i]]++;
        }
        int j = 0;
        for (int i = 0; i < k; i++) {
            int tmp = counters[i];
            while (tmp--) {
                tab[j++] = i;
            }
        }
        free(counters);
        return 0;
    }