Word Sort

Hello!

I have written this code which is supposed to take five names as input and then arrange them in alphabetical order. It got compiled easily but after I input those 5 names, the window closes automatically even when I have getch() in place. Also, I don't think I have written the function which sorts the names [word_swap()] any good.

Please help me set this code right....(I know ive made a mess, sorry for that :p )

Thanks!

Code:

---

#include<stdio.h>
#include<conio.h>
void word_swap(char*,char*);  /*I don't think I have written the definition of this function correctly*/
int main()
{
    char temp[20],names[5][20];
    int x,y,z;
    for(x=0;x<5;x++)
    {
                    printf("\nEnter a name :");
                    scanf("&#37;s",&temp);
                    for(y=0;names[x][y]!='\0';y++)
                    {
                                                names[x][y]=temp[y];
                    }
                    names[x][y]='\0';
    }
   
    for(x=0;x<5;x++)
    {
            for(z=x+1;z<5;z++)
            {
                              for(y=0;names[x][y]!='\0'&& names[z][y] !='\0';y++)
                              {
                                                          if((int)names[x][y]>(int)names[z][y])
                                                          word_swap(&names[x][0],&names[z][0]);
                              }
            }
    }
    for(x=0;x<5;x++)
    {
                   
                    printf("\n");
                    for(y=0;names[x][y]!='\0';y++)
                                                  printf("%c",names[x][y]);
    }
   
    getch();
    return 0;
}


void word_swap(char*str1,char*str2)
{
    int n=0;
    char temp;
    while(n!=2)
    {
              temp=*str1;
              *str1=*str2;
              *str2=temp;
                str1++;
                str2++;
                if(*str1=='\0')
                              n++;
                if(*str2=='\0')
                              n++;
    }
                                       
}

---

scanf is a bad choice for reading a string from user. It leaves characters in the buffer. In the best scenario is leaving the ENTER key which getch() is picking up.

By the way, temp is a pointer to the first element of the subscript when you pass it to scanf, so you don't need the &.

Better to use fgets instead of scanf to read strings. Scanf can be dangerous when reading strings as well. See my signature.
And you could really use some less spaces for each indentation level. 4 is more than enough.

Quote:

---

Originally Posted by Aia

temp is a pointer to the first element of the subscript when you pass it to scanf, so you don't need the &.

---

Thanks for that :). I wonder why it didn't give an error.

I replaced

Code:

---

scanf("%s",&temp);

---

with

Code:

---

fgets(temp,20,stdin);

---

But the window is still closing as soon as I input all the five names.

Have i written the word_swap() definition correctly ?

Post your updated code please.

Quote:

---

Originally Posted by abk

Have i written the word_swap() definition correctly ?

---

If you are referring to the prototype void word_swap( char *, char * ); Yes it it correct. The compiler only needs to know that
word_swap accepts two string pointers.

fgets() will read up to 19 characters and it will insert a '\0' at the end of the string.
some possibilities:
You enter a name sorter or up to 18 characters, fgets reads everything including the ENTER key (newline) and attachs a terminator '\0' at the ending.

You enter a name exactly 19 long and press enter. fgets reads those characters, terminates it with '\0' and leaves ENTER key behind for the next call to pick it up.

You enter a name longer that 20 characters. Any thing beyond 19 characters is left in the buffer input, including the ENTER key.

BTW, I am assuming you are practicing loops and accessing subscripts of arrays. Because there's already standard functions that will do the same thing you're trying to achieve.

After you find what's the problem closing your program right away, we'll talk about why your code will not do what you intend.

Updated code

Code:

---

#include<stdio.h>
#include<conio.h>
void word_swap(char*,char*);  /*I don't think I have written the definition of this function correctly*/
int main()
{
    char temp[20],names[5][20];
    int x,y,z;
    for(x=0;x<5;x++)
    {
                    printf("\nEnter a name :");
                    fgets(temp,20,stdin);
                    for(y=0;names[x][y]!='\0';y++)
                    {
                                                names[x][y]=temp[y];
                    }
                    names[x][y]='\0';
    }
   
    for(x=0;x<5;x++)
    {
            for(z=x+1;z<5;z++)
            {
                              for(y=0;names[x][y]!='\0'&& names[z][y] !='\0';y++)
                              {
                                                          if((int)names[x][y]>(int)names[z][y])
                                                          word_swap(&names[x][0],&names[z][0]);
                              }
            }
    }
    for(x=0;x<5;x++)
    {
                   
                    printf("\n");
                    for(y=0;names[x][y]!='\0';y++)
                                                  printf("&#37;c",names[x][y]);
    }
   
    getch();
    return 0;
}


void word_swap(char*str1,char*str2)
{
    int n=0;
    char temp;
    while(n!=2)
    {
              temp=*str1;
              *str1=*str2;
              *str2=temp;
                str1++;
                str2++;
                if(*str1=='\0')
                              n++;
                if(*str2=='\0')
                              n++;
    }
                                       
}

---

Quote:

---

Originally Posted by Aia

If you are referring to the prototype void word_swap( char *, char * ); Yes it it correct.

---

No I am talking about this code:

Code:

---

void word_swap(char*str1,char*str2)
{
    int n=0;
    char temp;
    while(n!=2)
    {
              temp=*str1;
              *str1=*str2;
              *str2=temp;
                str1++;
                str2++;
                if(*str1=='\0')
                              n++;
                if(*str2=='\0')
                              n++;
    }
                                       
}

---

Code:

---

                    for(y=0;names[x][y]!='\0';y++)
                    {
                                                names[x][y]=temp[y];
                    }
                    names[x][y]='\0';

---

This won't work. The names array doesn't contain anything when you declared it, so it doesn't make sense to use what names[x][y] contains as a condition for the loop.

What might be a better solution is to read five names and then copy them into the names array explicitly with a string_copy function like strcpy(). You can write your own strcpy() if you want to, it's really easy. Just assign each character from the source to the destination until you reach the source's '\0'. Then assign '\0' to the destination.

Then you can focus on sorting. word_swap is ok, but only when str1 and str2 are the same length.

Come on, abk, you use way too many spaces for indentation. Lessen it a bit. It makes it look nicer and people don't have to scroll to read the code. It's very simple.

Your lexicographical ordering is wrong.
word_swap should not be within 3 nested loops. Right now it might swap two words many times over. The inner loop should be replaced with a strcmp.
Then you can find out what order the words should be in and THEN decided ONCE whether to swap them or not.

If I may suggest.
Write small pieces of code and compile often.
e.g
Write the part that will get the input into an array of strings. ( It is not necessary to go through a temporal string variable )
Compile.
Write a test loop that will display each string
Compile.
Notice the unwanted newline in each string.
Write the part that will eliminate the newline in each string.
Compile.
In a different file write a function that swap strings
Compile.
Test the function. If is bug free, add to the project
writing the part that loops through the strings and compare them.
Compile.
Debugging will be easier when you have tested every little portion and know that is working the way intended.

BTW You don't need #include<conio.h> and getch. A simple standard getchar() would do very well.

Quote:

---

Originally Posted by Elysia

Come on, abk, you use way too many spaces for indentation. Lessen it a bit. It makes it look nicer and people don't have to scroll to read the code. It's very simple.

---

Sorry for that, but its my IDE's fault. It indents automatically. I will take care abt it next time i post.