Help debug palindrome code

Hi guys!

I am learning C and am writing code to detect palindrome to help me learn to manipulate strings and use pointers and such. I am having trouble debugging it and was hoping you might be able to help.

Code:

---

void removeChar( char * string, char letter );
char* palindrome(char)

int main(void) {
        char string[100];

        printf("Enter what you think is a palindrome (w/o spaces): ");
        fgets(string, sizeof(string), stdin);
        palindrome(string);
        return(0);
}

int palindrome(string) {
        char string[100];
        int n, *f, *l;                                        //*f and *l are pointers to the first and last indexed character in string

        f=&string[0];                                        //do I need the ampersand since it is an array?
        l=&string[n];
        n = strlen(string)-1;
        if(string == " ")                                        //Base case for recursion
                printf("Yep! Thats is a palindrome!");

        else if(string[0] == string[n])
                palindrome = removeChar(string, *f);        //Can I use this format instead of a character in single quotes since the
                palindrome = removeChar(string, *l);  //dereferenced pointers point to a singe char?
                return(palindrome(string));                        //check to see if the next outside characters are equal
}

void removeChar( char * string, char letter ) {
        unsigned int i;

        for(i = 0; i < strlen( string ); i++ )
                if( string[i] == letter )
                        strcpy( string + i, string + i + 1 );
}

---

Quote:

---

Originally Posted by mckinnley

Hi guys!

I am learning C and am writing code to detect palindrome to help me learn to manipulate strings and use pointers and such. I am having trouble debugging it and was hoping you might be able to help.

Code:

---

void removeChar( char * string, char letter );
char* palindrome(char)

int main(void) {
        char string[100];

        printf("Enter what you think is a palindrome (w/o spaces): ");
        fgets(string, sizeof(string), stdin);
        palindrome(string);
        return(0);
}

int palindrome(string) {
        char string[100];
        int n, *f, *l;                                        //*f and *l are pointers to the first and last indexed character in string

        f=&string[0];                                        //do I need the ampersand since it is an array?
        l=&string[n];
        n = strlen(string)-1;
        if(string == " ")                                        //Base case for recursion
                printf("Yep! Thats is a palindrome!");

        else if(string[0] == string[n])
                palindrome = removeChar(string, *f);        //Can I use this format instead of a character in single quotes since the
                palindrome = removeChar(string, *l);  //dereferenced pointers point to a singe char?
                return(palindrome(string));                        //check to see if the next outside characters are equal
}

void removeChar( char * string, char letter ) {
        unsigned int i;

        for(i = 0; i < strlen( string ); i++ )
                if( string[i] == letter )
                        strcpy( string + i, string + i + 1 );
}

---

---

I've never used C, but looking at your code, I notice that your palindrome function returns an int in the function signature yet, it never actually returns an int?

For a palindrome, I would use a boolean function.

Something like: (not written in C)

Code:

---


bool isPalindrome(string s)
{
    for(int i = 0; i < s/2; ++i){
          if(s[i] != s[s.size]-i-1)
                return false;
         
        }
    return true;
}

---

This works by checking each character from the first half of the string with its equivalent in the 2nd half. If it finds a character that doesn't match, it returns false. Otherwise it will loop through and then return true.

As Darren points out, you're not returning anything from your palindrome function. That should give you at least a warning, and probably an error, from your compiler - check your settings on your compiler.

In your else statement, you are indenting the code as if you had used curly braces, but you did not use those braces, so it's very deceiving.

With decent attention to your indentation, you'll see the problems in your logic.

That's right, you don't need the ampersand, because the string array name is the address for the base of the array. That's what you want for f, but not for l. For l=&string[n], to work as you expect, you WILL need the ampersand. Good catch, Quzah.

I hope everyone gives you a chance to work this problem through - it's really a good one, without giving your the answers, in code.

Code:

---

int palindrome(string) {
        char string[100];
        int n, *f, *l;                                        //*f and *l are pointers to the first and last indexed character in string

        f=&string[0];                                        //do I need the ampersand since it is an array?
        l=&string[n];

---

What is the value of n? Yes, you actually do need the & here, because otherwise you are assigning a value - the value at string[ ? ], not the address-of that location.

Quote:

---

if(string == " ")

---

Those should be single quotes if you are trying to compare a single character, otherwise, you need to use strcmp or something like it if you want to compare the entire string to something.

Quzah.

I can either keep my [CODE]printf(yep thats a palindrome!) /CODE] statement and just add a return(0) and keep it declared an int or I could declare the palindrome function to return a char and use

Code:

---

printf("%s", palindrome(string))

---

. Either way works.

It seems like it doesn't matter which way I do it. Is there a safer way? Also, when would I declare the palindrome function to return char as opposed to char*? char* function() says that the function points to characters while char function() says that the function returns characters directly. So when is it appropriate to use which?

The calling function obviously has the string you're testing whether it's a palindrome or not, so there is no reason to return the string.

Also, there is no sense in returning a char from palindrome(), because no char is any more important than any other char. Either the string is a palindrome, or it isn't - which is a perfect boolean (0 or 1) type of return.

0 = not a palindrome
1 = it's a palindrome

The general Golden Rule of Returns is never to return the address of any local variable. Although it might work (probably will), depending on your system to refrain from re-using that address for a few lines of code, if very very iffy. You never know just when the local variable memory will be re-used for something else, and cause the whole program to crash ignominiously, when you try to reference that address.

I don't understand the remove_char() function. Why do you have it in your program? Imo, you need main() to get the user's string to be checked, and one function to check the string to see whether it's a palindrome or not.

And that is all you should need. Pseudo code with a bit of C format, might look like this:

Code:

---

include files

int is_palindrome(char string);  //prototype

int main(void) {
  declare your char array for the string to be checked
  and the int for is_palindrome to return it's value into

  get the string from the user - fgets() is a great choice
  You can either overwrite the newline in the string (fgets
  always will put one in the string, if there is room for it),
  or just compare to the last letter before the newline char.

  call ok = is_palindrome(arrayname).
  if(ok) 
    print affirmative message
  else
    print negative message

  return 0
}
int is_palindrome(char arrayname) {
  declare left and right index integers or pointers (whichever you prefer)
  check the array[], either iteratively or via recursion, (whichever you prefer)
  return 1 if it's a palindrome, or zero if not.
}

---

Hope that helps, without giving it all away.

Quote:

---

Originally Posted by mckinnley

Code:

---

        l=&string[n];
        n = strlen(string)-1;

---

---

What do you expect to happen when executing these two statements in the order shown?

Okay so I've been working on it and broke it all down and got this which works beautifully. My next step was to add a do-while like:

Code:

---

        do{
        printf("Enter what you think is a palindrome (w/o spaces): ");
        fgets(string, sizeof(string), stdin);
        }while(string =! "\n");

---

so that it will continue to prompt the user unless they hit enter. The problem is "string" is a character array and \n I thought would be considered a character but it must be reading it as an int because my error message is that they are "incompatible types". I don't understand how to change this. Insight?

Code:

---

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

int main(void) {
        char string[100];
        int i, len, pal;

        printf("Enter what you think is a palindrome (w/o spaces): ");
        fgets(string, sizeof(string), stdin);

        len = strlen(string)-1;

        for(i=0; i<=len/2; i++)
                if(string[i] == string[len-1-i]) {
                        pal = 0;
                }
                else {
                        pal = 1;
                }
        if(pal == 0) {
                printf("Yes it is a palindrome! \n");
        }
        else if(pal == 1) {
                printf("Not a palindrome");
        }
        else {
                printf("Something is wrong");
        }
}

---

The next step is where I am getting stuck. To finish the program I wanted to put palindrome into a function like below:

Code:

---

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

int palindrome(char string);

int main(void) {
        char string[100];

        printf("Enter what you think is a palindrome (w/o spaces): ");
        fgets(string, sizeof(string), stdin);

        if(palindrome(string) == 0) {
                        printf("Yes it is a palindrome! \n");
                }
                else if(palindrome(string) == 1) {
                        printf("Not a palindrome");
                }
                else {
                        printf("Something is wrong");
                }
}

int palindrome(char string) {
        int i, len, pal;

        len = strlen(string)-1;
       
        for(i=0; i<=len/2; i++)
                if(string[i] == string[len-1-i]) {
                        pal = 0;
                }
                else {
                        pal = 1;
                }
        return(pal);
}

---

but am getting errors that palindrome makes integer from pointer w/o cast. Does my declaration of palindrome need to be char* string to say that string points to a string?

You can't compare strings like that. You need something like strcmp, or to compare individual characters.

Quzah.

Yes, you need to pass string like it's a string - not a single char:

Code:

---

int palindrome(char *string); //is your prototype
palindrome(string); //is your call to the function

---

char's are just a small range integer, which is why you're getting that error message.

Then, in the else statement of the palindrome, you need to have:

Code:

---

else {

  return 1;
}

  return 0;

---

In the else part, you have to return with failure the very first time that the char checking shows the string is NOT a palindrome.

Otherwise the logic can fail if it finds a bad (non-matching) char, but followed by matching chars, in the string array being tested.

You're getting the hang of this! :D

I see, I see. I understand. Thank you so much for your help!

WHOOO! It works beautifully!

Code:

---

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

int palindrome(char *string);

int main(void) {
        char string[100];

        printf("Enter what you think is a palindrome (w/o spaces): ");
        fgets(string, sizeof(string), stdin);

        if(palindrome(string) == 0) {
                        printf("Yes it is a palindrome! \n");
                }
                else if(palindrome(string) == 1) {
                        printf("Not a palindrome");
                }
                else {
                        printf("Something is wrong");
                }
        return(0);
}

int palindrome(char *string) {
        int i, len, pal;

        len = strlen(string)-1;

        for(i=0; i<=len/2; i++)
                if(string[i] == string[len-1-i]) {
                        pal = 0;
                }
                else {
                        pal = 1;
                }
        return(pal);
}

---

I would write this:

Code:

---

        if(palindrome(string) == 0) {
                        printf("Yes it is a palindrome! \n");
                }
                else if(palindrome(string) == 1) {
                        printf("Not a palindrome");
                }
                else {
                        printf("Something is wrong");
                }

---

as:

Code:

---

        int result = palindrome(string);
        if (result == 0) {
                puts("Yes it is a palindrome!");
        }
        else if (result == 1) {
                puts("Not a palindrome");
        }
        else {
                puts("Something is wrong");
        }

---

Also, your palindrome checking code is wrong. For example, try "iMalc" and "darren" as the input. Your program will declare that both inputs are palindromes, but you can determine for yourself that they are not. The problem is that you should declare that the input is not a palindrome as soon as you discover that corresponding characters do not match. What you did instead was to declare that the input is or is not a palindrome depending only on the middle character(s).