Simple Palendrome Problem [newbie to C]

[colinuk]

Hi All,

Im trying to write a little programe that checks if a word entered is a palendrome or not.

The programe runs but gives in-correct results, im realy stumped as to whats up, I wondered if any one could point me in the right direction.
Many thanks
Colin

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

void main()

{
char mystring[20],compstring[20];
int length,loop,index=0;


printf("Please enter a word under 20 letters to test : ");
gets(mystring);

length = strlen(mystring)-1;

   for(loop=length;length>0;length--)
   {
   compstring[index] = mystring[loop];
   index++;
   }

   if(!strcmp(mystring,compstring))
   {
   printf("The string Is A Palendrome");
   }
   else
   {
   printf("The string Is NOT A Palendrome");		      
   }

}

[itsme86]

You're not nul-terminating compstring[] anywhere.

Also:
- Don't use void main
- Don't use gets
- Your spacing is atrocious

[colinuk]

Hi,
Thanks for your reply, but to be honest in not sure i follow, how do I nul-terminating compstring[] ?

I have edited the code and it just looks awful I agree, it looks great in my compiler but when I pasted it in ????

Thanks again
Colin

[bithub]

for(loop=length;length>0;length--)
{
	compstring[index] = mystring[loop];
	index++;
}

Look carefully at that code you have. Which variable is the for loop decrementing each iteration? Is that variable used in the for loop block?

[itsme86]

A string is an array of characters followed by a null-terminating 0 (ASCII value 0). So in memory a string looks like r a c e c a r \0

In your loop you're starting at strlen(mystring)-1 which is avoiding the \0 at the end of the loop and that's fine. But after the loop, compstring is left with everything except the \0. The only way functions like strcmp() know how big a string is is by running into that \0 as it's iterating through it. So after your loop you need to add that \0 to the end of compstring. That (along with decrementing the wrong variable in your for loop) should fix your problem.

[hk_mp5kpdw]

Incorporating suggestions from everybody here:

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

int main()
{
    char mystring[20],compstring[20];
    int length,loop,index=0;

    printf("Please enter a word under 20 letters to test : ");
    fgets(mystring,sizeof(mystring),stdin);

    if( mystring[strlen(mystring)-1] == '\n' ) mystring[strlen(mystring)-1] = 0;
    length = strlen(mystring)-1;

    for(loop=length;loop>=0;loop--)
    {
    compstring[index] = mystring[loop];
    index++;
    }
    compstring[index] = 0; // Null terminate the string

    if(!strcmp(mystring,compstring))
    {
    printf("The string Is A Palendrome");
    }
    else
    {
    printf("The string Is NOT A Palendrome");		      
    }

}

[edit]Forgot some things, fgets will also extract and store the newline character so allowances must be made to take care of that and also the > in the for loop should be >= so that it copies over an extra character. See changes above in [/edit]

[itsme86]

Or it could be done without a second array:

#include <stdio.h>

int main(void)
{
  char word[20];
  char *start, *end;

  printf("Please enter a word under 20 letters to test: ");
  fflush(stdout);
  fgets(word, sizeof(word), stdin);

  for(start = end = word;*end && *end != '\n';++end)
    ;
  *end-- = '\0';

  while(start < end)
  {
    if(*start != *end)
      break;
    start++;
    end--;
  }

  printf("%s is%s a palindrome.\n", word, start < end ? " not" : "");

  return 0;
}

[colinuk]

Hi guys,

Thanks a lot for your comments,
I reviewed my code and im still not sure how to actualy code the null termination mentioned??

On the loop I made these changes after thinking about how it was working, but its still not quite right, sorry guys

for(loop=length;loop>0;loop--)
  {
  index=0;
  compstring[index] = mystring[loop];
  index++;
  }

[itsme86]

Well, you don't want to set index to 0 at the beginning of every loop iteration, just once before the loop begins. The thing that you're missing is after the loop completes, adding the null terminating zero after all the letters in the compstring array.

[hk_mp5kpdw]

Forgot a couple of things in my post above, see 2 more changes made in red.

[colinuk]

This does not work either its like my code it constantly says Not a palendrome no matter whats entered??

Incorporating suggestions from everybody here:

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

int main()
{
    char mystring[20],compstring[20];
    int length,loop,index=0;

    printf("Please enter a word under 20 letters to test : ");
    fgets(mystring,sizeof(mystring),stdin);

    if( mystring[strlen(mystring)-1] == '\n' ) mystring[strlen(mystring)-1] = 0;
    length = strlen(mystring)-1;

    for(loop=length;loop>=0;loop--)
    {
    compstring[index] = mystring[loop];
    index++;
    }
    compstring[index] = 0; // Null terminate the string

    if(!strcmp(mystring,compstring))
    {
    printf("The string Is A Palendrome");
    }
    else
    {
    printf("The string Is NOT A Palendrome");		      
    }

}

[edit]Forgot some things, fgets will also extract and store the newline character so allowances must be made to take care of that and also the > in the for loop should be >= so that it copies over an extra character. See changes above in [/edit]

[Stack Overflow]

Yes,

The first modification can be done simply with strrchr(), included in the string library. What this does is it finds the last occurrence of your character in the string. I started a thread about the easiest way to accomplish this task, though it came down to using strchr() or strrchr().

Here is a sample of how strrchr() could be used:

char *pch;

if (pch = strrchr(mystring, '\n'))
	*pch = '\0';

- Stack Overflow

[hk_mp5kpdw]

Try adding this right before your strcmp function call to debug what is going on:

printf("mystring \"%s\" has length %d\n"
       "compstring \"%s\" has length %d\n",
       mystring,strlen(mystring),
       compstring,strlen(compstring));

Both strings should be the same backwards and forwards and also have the same length.

[itsme86]

To debug try printf()'ing mystring and compstring. See if they're really coming up as you expect them to. If they both look right, then the problem is probably in the algorithm that determines if it's a palindrome. If they don't look right, then the problem is in the algorithm that creates compstring.

Also, if the strings look different, seeing how they're different is usually a great indicator of what the problem is.

[colinuk]

Thank you all for your great responce of help.

I finally got it with this code.

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

int main(void)

{
char mystring[20],compstring[20];
int length,loop,index=0;


printf("Please enter a word under 20 letters to test : ");
gets(mystring);
fflush(stdin);
length = strlen(mystring)-1;

for(loop=length;loop>=0;loop--)

   {
   compstring[index] = mystring[loop];
   index++;
   }
compstring[index]=0;

   if(!strcmp(mystring,compstring))
   {
   printf("\nThe string Is A Palendrome");
   }
   else
   {
   printf("\nThe string Is NOT A Palendrome");
   }

}