Sort working in main() but not in separate function

Question

I'm working on a homework problem for C and unix programming, and the teacher told us to write a sort function for an array in C.

I've got sorting working from some for loops in main, but the separate sort function we need doesn't work.

#include <stdio.h>
#include <stdlib.h>

int Sort(int arr[], int size){

    int i,j,a;
    for(i = 0; i < size; i++){
        for(j = i+1; j < size; j++){
            if(arr[i] > arr[j]){
                a = arr[i];
                arr[i] = arr[j];
                arr[j] = a;
            }
        }
    return arr;
    }
}

int main(){
    int a;
    int BAT[40];
    for(int i=0; i < 40; i++){
       BAT[i] = (float)(599)* ( (float)rand() / (float)RAND_MAX );
       printf("%d \n", BAT[i]);
    }
    printf(" the array should now be sorted \n"); 
    //Sort(BAT, 40); THIS IS THE FUNCTION CALL THAT DIDNT SEEM TO WORK SO I COPIED THE SORT OUT OF THE SORT FUNCTION TO TEST

    //THIS IS THE SORT CODE AND WHILE IT IS IN THE MAIN IT WORKS
    for(int i = 0; i < 40; i++){
        for(int j = i+1; j < 40; j++){
            if(BAT[i] > BAT[j]){
                a = BAT[i];
                BAT[i] = BAT[j];
                BAT[j] = a;
            }
        }
    }
    //END OF SORTING TEST
        for(int j=0; j < 40; j++){
            printf("%d \n", BAT[j]);
        }

I expect the Sort(BAT, 40) to sort the array which I then try to print but instead nothing seems to occur.

Answer 1

Your sort routine should have worked as written, but you are failing to enable warnings in your code and thus you are not allowing the compiler to help you fix the warnings and errors in your code -- that alone would have allowed your code to run just fine.

For instance, your compiler will tell you the exact line number, and many times the exact character in that line where the error or warning was detected, e.g.

bubblesortfn.c: In function ‘Sort’:
bubblesortfn.c:21:5: warning: return makes integer from pointer without a cast 
[enabled by default]
     return arr;
     ^

How can a return make an integer from a pointer?? Simple, you have your function attempting to return an integer array (int *), and you have your function declared int Sort. (you don't need to return anything, but you can simply fix it by changing your declaration to int *Sort (....)).

The remainder of the problems are simple syntax issues and unused variables (e.g. a in main()) that would be instantly flagged by your compiler -- listen to it. Let it help you write better code.

Always compile with warnings enabled, and do not accept code until it compiles cleanly without warning. To enable warnings add -Wall -Wextra -pedantic to your gcc/clang compile string. For clang, instead you can use -Weverything. For VS (cl.exe on windows), use /W3 (or use /Wall but you will get quite a few extraneous non-code related warnings). Read and understand each warning -- then go fix it.

As mentioned in my comment, don't use magic numbers in your code (except where absolutely required like with the fscanf field-width modifier). Instead, If you need a constant, #define one (or more), or use a global enum to do the same thing. That way you have one single place at the top of your code to change things if needed and you don't have to go picking through your declarations or loop limits to change things.

Literally, fixing the warnings identified and tidying things up a bit was all that was needed to get your code working and properly sorting the array in your Sort function (I also added a prnintarray() function to print your array to avoid the repeated loops in main(). Putting it altogether, and seeding the random number generator by calling srand() before you use rand(), you could do:

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

/* if you need a constant, define one (or more) - avoid magic numbers */
#define ROWSZ   10      /* max integers to print per-row */
#define MAXB    40      /* max integers in BAT */
#define MULTP  599      /* multiplier constant */

int *Sort (int arr[], int size)
{
    int i, j, a;
    for (i = 0; i < size; i++) {
        for (j = i + 1; j < size; j++) {
            if (arr[i] > arr[j]){
                a = arr[i];
                arr[i] = arr[j];
                arr[j] = a;
            }
        }
    }
    return arr;
}

/* simple print function to output arr of sz with rowsz int per-row */
void prnintarray (int *arr, size_t sz, size_t rowsz)
{
    for (size_t i = 0; i < sz; i++) {
        if (i && i % rowsz == 0)
            putchar ('\n');
        printf (" %4d", arr[i]);
    }
    putchar ('\n');
}

int main (void) {

    int BAT[MAXB] = {0};    /* initialize all arrays - good practice */

    srand (time(NULL));     /* seed the random number generator */

    for (int i = 0; i < MAXB; i++)      /* fill array */
        BAT[i] = MULTP * ( (float)rand() / (float)RAND_MAX );

    puts ("\nunsorted array:\n");
    prnintarray (BAT, MAXB, ROWSZ); 

    Sort (BAT, MAXB);

    puts ("\nsorted array:\n");
    prnintarray (BAT, MAXB, ROWSZ); 
}

Example Use/OUtput

$ ./bin/bubblesortfn

unsorted array:

  461  519  346  508  265   93  358  407  278  151
  465  531  430  148  181  227  452  206  401  202
  103  518  259  267  342  495  570  431  477  455
  164  339  375  511  248   42    6    8  450  284

sorted array:

    6    8   42   93  103  148  151  164  181  202
  206  227  248  259  265  267  278  284  339  342
  346  358  375  401  407  430  431  450  452  455
  461  465  477  495  508  511  518  519  531  570

Look things over and let me know if you have further questions.

Use qsort For Real-World Sorting

While there is nothing wrong with writing a bubblesort function for the learning aspect of it, the C-library provides qsort which can, and should, cover the majority of your sorting needs. Your only job in using qsort is to write a simple compare() function to tell qsort how to sort adjacent members of the array. The prototype for the compare() function usually sends new C programmers into a state of panic. The prototype is:

int compare (const void *a, const void *b)

Don't let it bother you. a and b are just pointers to the two members of your array currently being compared. Your job is to write the remainder of the function so that if the value pointed to by a:

• sorts before the value pointed to by b, a negative number is returned;
• is equal to the value pointed to by b, zero is returned and finally
• sorts after b a positive values is returned. (all just like strcmp).

To handle the fact that a and b are void pointers, you simply cast them to int pointers before dereferencing to make use of their values, e.g.

int compare (const void *a, const void *b)
{
    const int *pa = a,    /* a and b are pointers to elements being compared*/
              *pb = b;    /* in array, cast as required to proper type */

Since your array is int, a and b will be pointers-to int, you simply cast them to int *. Now you can access the values through the pointers (e.g. dereference *pa to get the value at the address held by pa).

Now to satisfy the return requirements the trivial solution would be:

     return *pa - *pb;

However, if the *pa is a large negative value and *pb is a large positive value, subtracting *pa - *pb can easily result in integer overflow and undefined behavior. Instead of a direct subtraction, by using two inequalities, chance of overflow can be eliminated while providing the needed return. Think through:

    return (*pa > *pb) - (*pa < *pb);

So putting your qsort function together and replacing your call to Sort with a call to qsort, you would rewrite your code as:

int compare (const void *a, const void *b)
{
    const int *pa = a,    /* a and b are pointers to elements being compared */
              *pb = b;    /* in array, cast as required to proper type */

    /*  inequality avoids overflow from subtracting 2 large values
     *  (x > y) - (x < y) , returns -1, 0, 1 (like strcmp) for 
     *  -1 -> x sorts before y,  0 -> x equals y,  1 -> x sorts after y
     */
    return (*pa > *pb) - (*pa < *pb);
}

Then

    qsort (BAT, MAXB, sizeof *BAT, compare);

Give it a try. As a bonus for large arrays, qsort will be Orders of Magnitude faster than a bubblesort (one of the slowest sorts for large arrays)