String sort in alphabetical order

Code:

---

#include<stdio.h>
#include<string.h>

int str_lphchck(char ss1[35],char ss2[35],int n)
        {
  if(ss1[n]==ss2[n]) str_lphchck(ss1,ss2,++n);
  else return ss1[n]>ss2[n]? 1 : 0;
  }

int main()
        {
  char name[100][35],temp[35];
  int i,j,k,l;
  l = 0;
        while(gets(name[l])!=NULL) l++;

        for(k=0;k<2;k++) for(i=0;i<l-1;i++) for(j=l-1;j>i;j--)
          {
      if((name[j][0]<name[j-1][0] && k==0) || (!str_lphchck(name[j],name[j-1],0) && k==1))
              {
        strcpy(temp,name[j]);
        strcpy(name[j],name[j-1]);
        strcpy(name[j-1],temp);
              }
      }
  for(i=0;i<l;i++) printf("%s\n",name[i]);

  return 0;
  }

---

I wrote a function to sort strings in alphabetical order using bubble sort.
The question is, can I make this code more efficient?
And also, the output of this code seems to still have errors.

(These are not necessarily in any kind of order.)
1. "Efficient" and "bubblesort" don't really go together.

2. I have no idea what k is doing. What's the point of making two complete bubblesorts, where the first one only looks at the first letter and the second one looks at the whole string? And for that matter, you should put the checks on k first, so that the complicated-looking str_lphchck doesn't get called if it doesn't need to be. (Remember that C short-circuits && and || by always doing the left one first, so make that the easy one.) But you're still doing two bubblesorts for ... well, I don't know why.

3. There is no reason why anything should ever be called str_lphchck. str_lphcheck would be a little better, assuming that is supposed to be "check" abbreviated at the end. But still. lphchck? What does that even mean?

4. There is no reason why str_lphchck should ever be written, since strcmp is part of the C library in string.h. (Although: bonus points for using the ternary operator. :) However, you lose the bonus points since > is guaranteed to return either 1 or 0 anyway, so you could just return the result of the comparison.)

5. gets is evil. What happens when you get something of 35 or more characters?

6. Putting three for statements on one line is just wrong. Don't ever do it again.

7. The mechanism of bubblesort (the for-loops on i and j) appears to be right, AFAICT.

This is one bubblesort, from "C by Dissection" by Kelley and Pohl

Code:

---

void bubblesort[int a[], int n] /* n is the size of a[] */
{
    int i, j;
 
    for( i = 0; i < n-1; ++i)
      for( j = n - 1 ; i < j; --j)
          if( a[j - 1] > a[j])
            swap( &a[j - 1], &a[j]);

---

A separate swap function was used here, but I usually just include it in the body of the bubblesort, instead of making such a trivial function.

Note that only two for loops are needed. This is always the case with bubblesort.

For ease of understanding your code, and showing it to others, I STRONGLY recommend you put each line of code, on it's own line of text. Coding style is very important because over the years of learning to program, we are also learning, without even thinking about it, lots of visual cues to assist in our efforts.

A coding style that is quite different will largely disable that learning, and leave us to work a lot harder to understand the code. Many volunteers will not do it - and professionals will wonder WTF is wrong with this programmer?

Employer's won't allow it, of course.

Bubblesort is a highly inefficient sorting algorithm, (when done right), and will always be that way. If your sorting is taking too long, and you have more than 1,000 items to sort, I'd advise Shellsort, Mergesort, Heapsort, or Quicksort. They are more difficult to understand, and can be wrecked badly by any slight uncareful tinkering with them, but the last three especially, are quite fast.

There was a thread about Heapsort here in the forum, just one or two weeks ago, with several examples. (Naturally, I'd recommend my code in that thread, :) :) )

Yeah, that's why I think it's not efficient, because it needs to do 2 bubble sorts.
The first one is to check the first letter, the second one is to check the whole string, I think I'll look for a way to do this in just one bubble sort.

- Thank you very much, thanks to you I learned something new about that logical && and || thing.
- Yeah, I think using strcmp is just far more efficient.
- This code still makes some errors with many inputs.
- I used gets only to test it, because using fgets is complicated with the last '\n' character ;)
- str_lphchck is actually str_alphabetcheck

Thank you very much for your advice, I'll try to improve my programming style.
The sorting algorithm that I understand completely is only bubble sort, but I'll try to learn the other methods, and also, I'll check your code. ;)

The reason you're having problems is that your comparator is broken (you need to return the result of the recursive call, so that the result makes it all the way back out). Just use strcmp instead.

Yeah, I see how much more easier using strcmp is...

Code:

---

#include<stdio.h>
#include<string.h>

int main()
        {
  char name[100][35],temp[35];
  int i,j,k,l=0;

        while(gets(name[l])!=NULL) l++;

        for(i=0;i<l-1;i++)
          for(j=l-1;j>i;j--)
                  {
              if(strcmpi(name[j],name[j-1])<0)
                      {
                strcpy(temp,name[j]);
                strcpy(name[j],name[j-1]);
                strcpy(name[j-1],temp);
                      }
              }

  for(i=0;i<l;i++) printf("%s\n",name[i]);

        scanf("%d",&i);

  return 0;
  }

---