CODEWITHSUNDEEP
c
smooth_smoothie
smooth_smoothie

Reputation: 1343

Sorting alphabetically in C using strcmp

I am trying to sort records (structs) by their names. And I'm using strcmp to swap to detect the alphabetical order. The swapping works fine, however it doesn't always sort the entire list. It always leaves some records in thier incorrect alphabetic order.

void sort_by_name(){
    printf("%d\n", counter);

    int i=0, j=0;
    patient *temp;

    for(;j<=counter;j++){
        for(;i<counter-1;i++){

            if(strcmp(pRecords[i]->name, pRecords[i+1]->name) > 0){

                temp = pRecords[i];
                pRecords[i] = pRecords[i+1];
                pRecords[i+1]=temp;

            }//if loops

        }//2nd for loop

    }//1st for loop

}


counter-- number of records in the system.

Upvotes: 2

Views: 21477

Answers (2)

paxdiablo
paxdiablo

Reputation: 882146

I think your specific problem is that you're not re-initialising i to 0 each time you start the inner loop.

This is a relatively simple bubblesort where the inner loop runs over the list of N items, N times (controlled by the outer loop) to get them sorted. But your inner loop runs over them once then seems to go off elsewhere, beyond the end of the array. That's unlikely to provide useful results :-)

Your more general issue is: why are you not using the language facilities to do this, specifically qsort? If you're trying to educate yourself on sorting, that's fine, but if you just want to sort, there's little point re-inventing the wheel.

For what it's worth, I prefer the following algorithm in bubble sort. It stops early if the list is sorted and it doesn't look at already sorted items:

void sort_by_name(){
    int i, didSwap = 1, limit = numitems - 1;
    patient *temp;

    while (didSwap) {
        didSwap = 0;
        for (i = 0; i < limit; i++) {
            if (strcmp (pRecords[i]->name, pRecords[i+1]->name) > 0) {
                temp          = pRecords[i];
                pRecords[i]   = pRecords[i+1];
                pRecords[i+1] = temp;
                didSwap = 1;
            }
        }
        limit--;
    }
}

The didSwap variable controls when to exit. If you traverse the entire unsorted section without swapping then obviously, you're finished. The limit controls that unsorted section and reduces gradually since, after each pass, there's a greater already-sorted section at the end of the list (the higher elements bubble up to the end of the list, one per pass).

Upvotes: 3

Himadri Choudhury
Himadri Choudhury

Reputation: 10353

I think you are trying to implement bubble sort. The loop count variables seem to be a bit off.

        for(;i<counter-1;i++){

should be

        for(i=counter-2;i>=j; i--){

You must reset the i after every iteration of the 'j' loop. And work backwards to have the smallest number bubble up to the front.

Upvotes: 0

Related Questions