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

Bubble sorting in C

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

Problem

I've been into algorithms lately, so I wanted to apply a simple algorithm using a language of my choice, which was C in this case.

I've implemented the bubblesort algorithm (for strings) in a simple program:

#include 
#include 
#include 

#define NUM_NAMES (5)

void sort(char ** sorted, char ** strs, const size_t size, const bool ascending) { // using the bubble sort algorithm
    sorted[0] = strs[0];
    char ** f = strs;
    for(int u=0; u = 0) { // one condition would do. Only to be thread-safe
            reversed[i1] = sorted[i2];
            i1++;i2--;
        }
        for(int i = 0; i < size; ++i)
            sorted[i] = reversed[i]; // putting it to sorted
    }
}

void printNames(char * q, char ** names, int num) {
    printf("\t%s\n", q);
    for(int i = 0; i < num; ++i)
        printf("%d: %s\n", i+1, names[i]);
    for(int i = 0; i < 30; ++i)
        printf("=");
    printf("\n");
}

int main(int argc, char const *argv[])
{
    char * names[] = {
        "This should be Second",
        "This should be First",
        "This should be before the last",
        "Wait .. That's the last!",
        "This should be Third"
    };

    char *names_ordered[NUM_NAMES];
    printNames("Original", names, NUM_NAMES);
    sort(names_ordered, names, NUM_NAMES, true);
    printNames("Ascending", names_ordered, NUM_NAMES);
    sort(names_ordered, names, NUM_NAMES, false);
    printNames("Descending", names_ordered, NUM_NAMES);
    return 0;
}


I want to know if there's a problem with the sort function, especially in the reversing part, because I think that that's not efficient.

Solution

Reversing is not very efficient indeed (but who cares about an extra linear pass when bubble sort itself is quadratic?). I would rather account for the requested order during the comparison:

result = strcmp(...);
if (!ascending)
    result = -result;


  • Initialization f = strs is very confusing, because later on f is reinitialized to sorted. I'd initialize it to sorted always, as close to use as possible.



Something like

for(int u=0; u < size; ++u) {
    char ** f = sorted;
    for(int i = 0; i < size - 1; ++i) {
        ...
    }
}


  • One-character names, especially unmotivated like f, u and q should be avoided. You really have to figure out what the variable is, and name it accordingly.

Code Snippets

result = strcmp(...);
if (!ascending)
    result = -result;
for(int u=0; u < size; ++u) {
    char ** f = sorted;
    for(int i = 0; i < size - 1; ++i) {
        ...
    }
}

Context

StackExchange Code Review Q#61904, answer score: 16

Revisions (0)

No revisions yet.