Problem with alphabetical sorting the strings from arrays....

[smith32]

Hi, I need some help from u again..... if we type:

Enter Input : eee ddd cccc

the result should be like this :

sorting : ddd eee cccc

but actual result is like this :

sorting : eee eee cccc

I don't know why... Can u help me ?? Here is my code....

void wordSorter()
{ 
   char input[60], word[10][30], *temp;
   int i, count= 0, j = 0, sort1, sort2;

   printf("\n4) Word Sorter\n");
   printf("--------------\n");
	
   printf("Enter string(s) (1 - 60 characters): ");
   
   fgets(input, 61, stdin);
   
   if(strlen(input) >= 60) readRestOfLine();

   i = strlen(input)-1;
   if( input[ i ] == '\n') input[i] = '\0';
   
   for( i = 0; i <= strlen(input) && input[i] != '\0' && input[i] != '\n'; i++)
   {
    if(input[i] == ' ') {word[count][j] = '\0';count++;j = 0;} 
    else {word[count][j] = input[i];j++;}
    word[count][j] = '\0';
   }
    
   for(sort1 = 0; sort1 <= count; ++sort1)
      for(sort2 = sort1 + 1; sort2 <= count; ++sort2)
         if(strcmp(word[sort1], word[sort2]) > 0)
         {
            temp = word[sort1];

           
            strcpy(word[sort1],word[sort2]);

            strcpy(word[sort2],temp);
          
         };
   printf("\nList of unique words: ");
   for(i = 0; i<= count; i++)
    printf("%s ",word[i]);
   
   
  ...
}

[Auraomega]

From the top...

[Line 4] Why are you using a 2d array as well as a 1d array?

[Line 12] Getting data 1 longer than the array (2 longer than you should be using seeing as the last value of a char array should be 0x00).

[Line 19] Why are you doing strlen in every loop? First thing is strlen won't change, second thing it's a costly function, third thing is input != '/0' should do the check you need (when you fix line 12 that is).

[Line 30] (This is the line that is causing your visible issue) You've declared temp as a pointer to char, so you're not actually giving it the value in word[sort1], simply the address to work[sort1], and when you change the content you've not got a back up. Change line 4 to make temp a char not pointer to char.

[Line 33] Why are you using strcpy to copy a single byte? word[sort2] = word[sort1] would do the trick.

There may be other issues but I'm reading on my netbook so may have overlooked some things. If you have any further issues post back and I'll try and help more.

[smith32]

Thz for ur help...
but I'm not clear for line 12 should be 62 or just 60?

[smith32]

I change Line 30 and Line 33 to word[sort2] = word[sort1]; char temp[10]; but it's not work..... error is incompatible type of assignment ..... why?? I'm sorry coz I'm a beginner...

[Auraomega]

>error is incompatible type of assignment ..... why??

Change line 4 to:

char input[60], word[60], temp;

This is going to 'break' a lot of your code you have, so be prepared to re-code certain parts of it. It is necessary though as it was too easy to abuse your previous arrays.

>but I'm not clear for line 12 should be 62 or just 60?

Line 12 to:

fgets(input, 59, stdin);

Unfortunately I'm not going to be able to offer any more help now as my main PC is having some major stability issues which I must attend to, with any luck one of the other members can help you until I've sorted out my problem. Good luck!

[Adak]

ddd ccc eee is not in sorted order, so I'm not sure if you want to sort in ascending or descending order.

You should keep the 2D array, no need to change that.

you can't use temp like you have it in the sort portion. When you assign it to the address for string1, that's fine.

But then you strcpy string2 into string1! Guess what temp is pointing to NOW?

Then you're putting string2 right back into string2 with the final assignment from temp to string2.

Print out your sort when it swaps, and see it. Put in a getchar() to stop it after every swap, if you don't step through it with watches set.

Better to make temp an array, instead of a ptr, imo.

[Auraomega]

>You should keep the 2D array, no need to change that.
If he keeps his current 2D array then putting in a string of chars without a space will overflow, granted it should still be kept within the boundaries of his array, but it'll overflow still and is bad practise.

[Adak]

No. ;)

He can use your fgets() suggestion, to limit the number of char's that can be input. There is no need to use the "input" array, AFAIK.

He should definitely keep the 2D word array, and it is certainly NOT "bad" practice.

He has some other code that is useless, such as the loop with "count", that isn't needed. Whether the string contains spaces or not won't matter with fgets():

int numNames, MaxLength=30;
printf("\n How many names do you want to enter? [1-10] ");
scanf("%d", &numNames);
getchar();  //remove the leftover '\n'
for(i=0;i<numNames;i++) 
  fgets(word[i], MaxLength-1, stdin);

After that, sorting the strings is straight forward. I pointed out his mistake earlier in his sorting.

If he has any problems I'll be glad to sort out his sort, again. ;)

[Auraomega]

printf("Enter string(s) (1 - 60 characters): ");

So he asks for 1-60 chars but:

word[10][30]

means that anything over 30 will overflow assuming we don't enter a space:

if(input[i] == ' ') {word[count][j] = '\0';count++;j = 0;}

>He can use your fgets() suggestion, to limit the number of char's that can be input.
He can limit it earlier on in his code, but then he can't take 60 chars. He needs to change the size of his array to avoid either overflowing, or not taking the data he's supposed to take. I'd consider this bad practise to be honest.

>I pointed out his mistake earlier in his sorting.
We both pointed out his pointer issue which will fix his immediate problem, however depending on how the user puts the data in, it's easy to abuse his buffers which I'm trying to help avoid now.

[Adak]

He doesn't need to change the size of his array - just use the code like I posted. There are very few words longer than 30 char's, but he can make it more than 30 if he needs it.

it's not bad practice - you're not seeing what I'm suggesting, or you wouldn't be harping on this. Didn't you see "MaxLength" in my code I posted earlier?

C'mon - tick tock, hop on board, train she is a'moving.

Yeah, I saw your good suggestion on the sort problem, but I had already posted before I read through your post.

You know as well as I do that if you absolutely want to save the maximum amount of space, and still get every long word, you would use a "ragged" 2D pointer array. It's just a bit above his skill level atm, and may be more than he needs, anyway.

[Auraomega]

I think we're both assuming different things here, you're assuming that he doesn't have to take 60 chars, and I'm assuming he has to.

I understand there are very few if any times when someone would put in a word more than 30 characters long, however this doesn't remove the point that assuming the OP has to take 60 chars (like in the case of course work) then keeping the buffers is open to abuse. Maybe I'm just being anal but I like to make sure there is no chance of my buffers overflowing. I'm also not saying he needs to change from a 2D array (it was an earlier suggestion to avoid confusion), but changing it to [10][60] to keep in line with his other code would avoid buffer overflow opportunities.

[Adak]

Using fgets(), with the 30-1, there is NO chance of a buffer over-run. None!

You can press keys until you're blue in the face, and it won't accept any more char's.

[Auraomega]

>you're assuming that he doesn't have to take 60 chars
>Using fgets(), with the 30-1, there is NO chance of a buffer over-run. None!

Thank you for proving my point.

[Adak]

I'm assuming he's not a brain dead yokel that just fell off the turnip truck yesterday, and will adjust MaxLength to whatever size he needs.

Of course.

[Auraomega]

And after raising his MaxLength to over 30; hello overflow.

[smith32]

Thank you for all of your help .... I have already solved the problem including all of your advice... Thank and I already take care for overflow by changing word[30][60] .... Thank you for all

[Adak]

And after raising his MaxLength to over 30; hello overflow.

#define MaxLength 30 //or 60, or whatever he needs?
or
const int MaxLength=30;  //or 60, or whatever.

and then:

word[10][MaxLength];

There can be data being truncated, but there can be no overflow as long as he uses MaxLength to set his size, in fgets().

After you put the string into the buffer with fgets(), you would get it's length in char's, with strlen(), and then malloc or calloc that many chars. You need stdlib.h for these functions.

Anything you malloc or calloc, you need to free(bufferName), or you run a risk of having a memory "leak" of lost memory, on that system. (depending on your OS and system).

[prvnkmr449]

Hi Adak in this i also get the same solution when i try to solve use array instead of pointer but you have any region for that please post it I want to known the region coz as I know this code have to work what ever we use array or a pointer, coz these are near to same.
thanks

[Adak]

Praveen, please start a new thread for your question, and have someone help you with the English OK?

I don't know what you are asking.