C: Sorting a list alphabetically

Question

I have problem with my project. I wrote a function that reads a struct from a file, sorts it alphabetically and writes back to the file. Reading from the file and putting it back to it is ok as I use the same code in other functions and it works perfect. There's something wrong with my sorting, as after using this function the txt file is blank. It's working on a structure outside the functions:

typedef struct baseofwords                                
    {
        char *word;
        char *category;
        struct baseofwords* next;
    } base;

Here's my function:

void SORTING (base **head)
{
char word[30];
char category[20];
FILE *fp;
if ((fp = fopen("baza.txt", "r"))==NULL)
    {printf("Error while opening the file!");
    exit(EXIT_FAILURE);}
else
    {
    while(!feof(fp))
        {
        fscanf(fp,"%s %s \n", word, category);
        base *wsk = *head;
        base *new = malloc (sizeof(base));
        new -> next = NULL;
        new -> word = strdup(word);
        new -> category = strdup(category);
        if(wsk == NULL)
            {
            new -> next = *head;
            *head = new;
            }
        else
            {
            while(wsk -> next != NULL)
            wsk = wsk -> next;
            wsk -> next = new;
            }
        }
    }
fclose(fp);
//==========================================up until here it works, problem must be down there


base *newHead = NULL;
base *wsk1, *wsk2, *tmp;
wsk1 = tmp = *head;
wsk2 = NULL;
while(tmp->next)
    { if (tmp->next->word > wsk1->word)
        { wsk2 = tmp;
        wsk1 = tmp->next;
        }
    tmp = tmp->next;
    }
if (wsk2) wsk2->next = wsk1->next;
else *head = wsk1->next;
wsk1->next = newHead;
newHead = wsk1;
*head = newHead;


//======================this part is okay again
if ((fp = fopen("base.txt", "w"))==NULL)
    {printf("Error while opening file!");
    exit(EXIT_FAILURE);}
else
    {base *wsk = *head;
    while (wsk->next != NULL)
    {fprintf(fp, "%s %s\n", wsk->word, wsk->category);
    wsk=wsk->next;}
    }fclose(fp);
}

Thank you very much in advance for help!

How does this line: base *new = malloc (sizeof(baza)); ? I don't even see how it compiles... — GreenAsJade, Jan 19 '14 at 00:14
Sorry! I translated it from my language. It's base *new=malloc (sizeof(base)); , of course! — user3188206, Jan 19 '14 at 00:15
I think it would be better if we were looking at the actual code that you are using, or even better ... if you compile and test the code you post here before you post it saying that "it works"... — GreenAsJade, Jan 19 '14 at 00:17
The best way to debug this would be to put print statements inside the loop, telling you what wsk1, temp and wsk2 are each time around. I think you will quickly find the errors that way, rather than waiting for some of us keen folk to deduce what you are trying to do... — GreenAsJade, Jan 19 '14 at 00:19
You are using feof incorrectly:http://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong — William Pursell, Jan 19 '14 at 00:33

woolstar · Answer 1 · 2014-01-19T00:24:11.913

Several things are wrong here, lets look at this loop:

while(tmp->next)
    { if (tmp->next->word > wsk1->word)
        { wsk2 = tmp;
        wsk1 = tmp->next;
        }
    tmp = tmp->next;
    }

You assign something to wsk2 and then never use it. You try and compare word by comparing its address (instead of the values at that address). And I don't see how you re-order items at all.

To compare two strings, you need to use strcmp, and you also need to decide what order you want them in.

Second, you can't sort a list just by stepping through it once. You are going to have to have nested loops that compare items with each other and perform possibly multiple moves to move the items into place.

And reordering items in a linked list (singlely linked no less), is rather difficult and fraught with edge conditions that have to be accounted for. You should consider instead moving the pointers to word and category when you want to swap two items. That might look something like:

void swapbases( base *it1, base *it2 )
{
  base tmp= * it1 ;
  it1-> word= it2-> word, it1-> category= it2-> category ;
  it2-> word= tmp.word, it2-> category= tmp.category ;
}

And then write a couple of loops that step through and find items out of order. There's a terrible one called bubble sort:

int swaps ;
for ( swaps= 1 ; ( swaps ) ; )
{
  swaps= 0 ;
  for ( tmp= head ; ( tmp-> next ) ; tmp= tmp-> next )
  {
     if ( strcmp( tmp-> word, tmp-> next-> word ) < 0 )
       { swapbases( tmp, tmp-> next ) ;  swaps ++ ; }
  }
}

Yeah, it looks like the code after the while{} is supposed to be inside it somehow (because it's this after code that uses wsk2) — GreenAsJade, Jan 19 '14 at 00:18
Thank you very much for explaining me everything patiently! Now I see my mistakes. I removed the part in and after the while{} and added what you told me. Now it saves the list correctly, but it's still unsorted. — user3188206, Jan 19 '14 at 00:56

C: Sorting a list alphabetically

1 Answers1