Why is the answer submitted wrong

Question

#include<stdio.h>
void f(char *s[],int n)
{
    int i=0;
    int j=0;
    int x=0;
    int k=i;
     
       for (x=0;x<n-1;x++)
        {
            for(j=0;j<n-1;j++)
                {
                if(*(*(s+j)+i)>*(*(s+j+1)+i))
                   {
                    char *temp;
                    temp=*(s+j);
                    *(s+j)=*(s+j+1);
                    *(s+j+1)=temp;
                     }
                else
                    if(*(*(s+j)+i)==*(*(s+j+1)+i))
                    {
                      for(k=1;k<n-1;k++)
                      {
                          if(*(*(s+j)+k)>*(*(s+j+1)+k))
                       {
                          {
                              char *temp;
                              temp=*(s+j);
                              *(s+j)=*(s+j+1);
                              *(s+j+1)=temp;
                              break;
                          }
                       }
                      }
                    }
                }
        }
}
int main()
{
    int n=0;
    char * str[100];
    char a[100][100];
    while(n<=100&&gets(a[n])!=NULL)
    {
        str[n]=a[n];
        n++;
     }
    f(str,n);
    int i=0;
     for(i=0;i<n;i++)
    {
        puts(str[i]);
    }
    return 0;
}

That's the question of this code:Input multiple English words to the string array, in alphabetical order from small to large output and you can't use strcmp.

The sample input: one
two
three
four

Sample output : four
one
three
two

My question: What's wrong with this code

Do try to use more meaningful variable names so that the reader can understand your code. Also, do you mean, arrange the strings in ascending order, because the question you are asking is a bit unclear? If so, just use strcmp to get which order the strings should be and use any normal sorting algorithm — Shravya Boggarapu, Dec 17 '21 at 13:37
@sudafin This while loop while(n<=100&&gets(a[n])!=NULL) can invoke undefined behavior when n is equal to 100. — Vlad from Moscow, Dec 17 '21 at 13:41
*Never* use `gets`: https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used — William Pursell, Dec 17 '21 at 13:43
@sudafin This for loop for(k=1;k*(*(s+j+1)+k)) { also can invoke undefined behavior due no using the condition k < n - 1 because the length of a string can be much less than the value of n -1. — Vlad from Moscow, Dec 17 '21 at 13:43
The main reason why people don't like the code is the pointless re-direction and pointer arithmetic. Don't write unreadable stuff like `*(*(s+j)+i)`, write `s[i][j]`. Also don't use `gets` since it has been obsolete for several decades and was finally removed from the C language one decade ago. — Lundin, Dec 17 '21 at 14:48
Programming is not an intellectual puzzle where you try to get a program that works using the minimum or cleverest set of characters that gets the compiler to do what you want it to do. Programming is *communication*, so a program must be *readable* by other humans. (And, of course, it must also get the compiler to do what you want it to do.) Why name your function `f()` when `sort()` would be so much clearer? Why write things like `*(s+j)` when `s[j]` would be so much clearer? — Steve Summit, Dec 17 '21 at 14:48
@SteveSummit Famous quote: “Everyone knows that debugging is twice as hard as writing a program in the first place. So if you're as clever as you can be when you write it, how will you ever debug it?" - Brian Kernighan — Lundin, Dec 17 '21 at 14:51
Regarding your function, `void f(char *s[], int n)` is wrong. `void f(char *s, int n)`, `void f(char s[], int n)` both are correct. — Abhishek Mane, Dec 26 '21 at 03:41
Please clarify your specific problem or provide additional details to highlight exactly what you need. As it's currently written, it's hard to tell exactly what you're asking. — Community, Dec 26 '21 at 03:44

Vlad from Moscow · Answer 1 · 2021-12-17T14:17:46.503

For starters declare variables in minimum scopes where they are used. That is instead of this code snippet

int i=0;
int j=0;
int x=0;
int k=i;
 
   for (x=0;x<n-1;x++)
   //...

You could just write

   for ( int x=0;x<n-1;x++)

Otherwise the code is difficult to read. For example this initialization

int k=i;

says nothing for the reader of the code because the variable k is overwritten in this for loop

for(k=1;k<n-1;k++)

You need not to separate comparisons of first characters of strings and next characters of strings in case when the first characters are equal. Such a separation again just confuses readers of the code. Instead you could use the standard function strcmp to compare two string.

This for loop

                  for(k=1;k<n-1;k++)
                  {
                      if(*(*(s+j)+k)>*(*(s+j+1)+k))
                   {
                      {
                          char *temp;
                          temp=*(s+j);
                          *(s+j)=*(s+j+1);
                          *(s+j+1)=temp;
                          break;
                      }
                   }
                  }

can invoke undefined behavior because length of the compared strings can be much less than the value of the expression n-1.

Instead of the for loop it is better to use the while loop

  k = 1;
  while ( *(*(s+j)+k) != '\0' && !( *(*(s+j)+k) > *(*(s+j+1)+k) ) )
  {
      ++k;
  }


  if( *(*(s+j)+k) != '\0' )
  {
      char *temp;
      temp=*(s+j);
      *(s+j)=*(s+j+1);
      *(s+j+1)=temp;
  }

Though it is much better to use the subscript operator instead of the dereferencing operator.

This while loop

while(n<=100&&gets(a[n])!=NULL)

can invoke undefined behavior when n is equal to 100 because the valid range for the arrays is [0, 100 ).

The function gets is unsafe and is not supported by the C Standard. It is better to use for example fgets.

I just learned it now, so I don't understand some of it. Thank you very much for your answer. I will study it carefully. — sudafin, Dec 17 '21 at 14:07

Why is the answer submitted wrong

1 Answers1