HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

A few sorting algorithms implemented in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
sortingimplementedfewalgorithms

Problem

I was reading about the elementary sorting algorithms like insertion Sort, selection sort and bubble Sort in Introduction to Algorithms by Cormen and I decided to try and implement it in C.

#include
#define MAXSIZE 100  //  Maximum number of elements permissible in array

void swap(int*,int*);
void print(int*,int);

int main(void) {
    int arr[MAXSIZE],n,i,j,ch,ord;
    void insertion(int*,int,int),selection(int*,int,int),bubble(int*,int,int);
    printf("\nEnter number of elements for array = ");
    scanf("%d",&n);
    printf("\n\nEnter the array elements one by one : \n\n");
    for(i=0;i0;j--){
            if((ord==1 && a[j]a[j-1]))
                swap(&a[j],&a[j-1]);
            else
                break;
        }
    }
}

void selection(int *a,int l,int ord){
    int index,i,j;
    for(i=0;ia[index]))
                index=j;
        }
    swap(&a[i],&a[index]);
    }       
}

void bubble(int *a,int l,int ord){
    int i,j;
    for(i=l;i>0;i--){
        for(j=0;ja[j+1]) || (ord==2 && a[j]<a[j+1]))
                swap(&a[j],&a[j+1]);
        }
    }
}

void print(int *a,int l){
    int i;
    for(i=0;i<l;i++)
        printf("%d\t",a[i]);
}


I'm looking for feedback / improvements on my code and whether the implementation of the sorting algorithms in the respective function definitions are proper or not.

Solution

This is a pretty good straightforward implementation of these algorithms. It's really nice that they're all implemented in-place so no extra memory is needed. It looks pretty good, but I have a few suggestions.

Better Naming

Your function names are pretty good. (I might add "sort" to the various sorting routine names, but it's not a big deal.) However, the variable names are a bit too terse. Normally, I'd object to using arr as the name for an array and name it after what's stored in the array. (Something like grades or names or whatever was being sorted.) In this example case, it's acceptable, but keep that in mind for the future.

It's tradition for i and j to be used for loop control variables, and I don't mind those. But I would change n to numElements. (Or if you have a better name for the array, name it num such as numGrades, numNames, etc.)

The function parameters a and l should be named with more readable names, like arr and length. ord is reasonable, but I'd prefer order.

Use enum when appropriate

ch is presumably supposed to hold a character, which would be a reasonable name, except that it holds an int. Likewise, ord is supposed to hold the order.

Both of those should be an enumerated type. I'd make one for the sort type and one for the sort order. Something like this:

typedef enum {
    SO_Ascending = 1,
    SO_Descending
} SortOrder;

typedef enum {
    ST_Insertion = 1,
    ST_Selection,
    ST_Bubble
} SortType;


You can then create variables of those types and read the values into them.

Error handling

You may have noticed that the user can enter invalid values, too. You should verify that the values they entered are reasonable and prompt them again if not. To do that, it would probably make sense to break out some of the functionality in main() into separate functions. I suggest something like this:

void promptForArray(int arr [ MAXSIZE ], int* numElements)
{
    do {
        printf("\nEnter number of elements for array = ");
        scanf("%d", numElements);
        if ((*numElements  MAXSIZE))
        {
            printf("\nYou entered an invalid value for the number of elements. Please choose a value between 1 and %d\n", MAXSIZE);
        }
    } while ((*numElements  MAXSIZE));

    printf("\n\nEnter the array elements one by one : \n\n");
    for(int i = 0; i < *numElements; i++){
        scanf("%d",&arr [ i ]);
    }
}


You could then do a similar thing for getting the sort function and sort order. Something like this:

void promptForSortType(SortType* sortType)
{
    do {
        printf("\n\nWhat sorting you want to use ?\n\n Insertion (1) , Selection (2) or Bubble (3)\n\nEnter choice = ");
        scanf("%d", sortType);
        if ((*sortType  ST_Bubble))
        {
            printf("\nYou entered an invalid sort type. Please enter a value between 1 and 3.\n");
        }
    } while ((*sortType  ST_Bubble));
}


Readability

Do the above things should improve the readability of your code, but you can do more. I recommend leaving spaces around operators (+, -, =, etc.).

This method of introducing function prototypes within main() is very odd:

void insertion(int*,int,int),selection(int*,int,int),bubble(int*,int,int);


Instead, you should put them above main and put them all on separate lines, and you should name the parameters so someone reading them knows what the parameters mean. Something like this:

void insertion(int *arr, const int length, const SortOrder ord);
void selection(int *arr, const int length, const SortOrder ord);
void bubble(int *arr, const int length, const SortOrder ord);


Use const where appropriate

You may have noticed that I made length and ord const in the above examples. They don't change, so it's best to mark them as unchanging. Then the reader and the compiler know more about the intent of the function. You could also make arr const in the print() function.

Code Snippets

typedef enum {
    SO_Ascending = 1,
    SO_Descending
} SortOrder;

typedef enum {
    ST_Insertion = 1,
    ST_Selection,
    ST_Bubble
} SortType;
void promptForArray(int arr [ MAXSIZE ], int* numElements)
{
    do {
        printf("\nEnter number of elements for array = ");
        scanf("%d", numElements);
        if ((*numElements <= 0) || (*numElements > MAXSIZE))
        {
            printf("\nYou entered an invalid value for the number of elements. Please choose a value between 1 and %d\n", MAXSIZE);
        }
    } while ((*numElements <= 0) || (*numElements > MAXSIZE));

    printf("\n\nEnter the array elements one by one : \n\n");
    for(int i = 0; i < *numElements; i++){
        scanf("%d",&arr [ i ]);
    }
}
void promptForSortType(SortType* sortType)
{
    do {
        printf("\n\nWhat sorting you want to use ?\n\n Insertion (1) , Selection (2) or Bubble (3)\n\nEnter choice = ");
        scanf("%d", sortType);
        if ((*sortType < ST_Insertion) || (*sortType > ST_Bubble))
        {
            printf("\nYou entered an invalid sort type. Please enter a value between 1 and 3.\n");
        }
    } while ((*sortType < ST_Insertion) || (*sortType > ST_Bubble));
}
void insertion(int*,int,int),selection(int*,int,int),bubble(int*,int,int);
void insertion(int *arr, const int length, const SortOrder ord);
void selection(int *arr, const int length, const SortOrder ord);
void bubble(int *arr, const int length, const SortOrder ord);

Context

StackExchange Code Review Q#117191, answer score: 3

Revisions (0)

No revisions yet.