I apologize that I cannot use English fluently because I am not a English speaker.
I want to get palindromes such as "A man, a plan, a canal, Panama!" or "Was it a car or a cat I saw?"
I studied Pointer in C today. So I made code to get palindromes.
#include <ctype.h>
/**
* @brief : Check if the given str is a palindrome
* @return: 1 if the given str is a palindrome, zero otherwise
* @param : str - pointers to the null-terminated byte string to check
*/
int ispalindrome(char *str)
{
int i, j;
char *pal;
for(i=0 ; *str!=0; i++) {
if(ispunct(*str)){ //if *str is punctuation mark, do not anything.
str++;
}
else if(isupper(*str)){ //if *str is a capital letter, change it to small letter and *pal=*str
*pal = tolower(*str);
str++;
pal++;
}
else //if *str is a small letter or a digit, *pal=*str
*pal = *str;
str++;
pal++;
}
for(j=0; pal[j]==pal[i-(j+1)] && j<=i/2; j++) //compare pal
;
if(j>i/2)
return 1;
return 0;
}
int main(void)
{
char buffer[1024];
int i;
gets(buffer);
printf("[%s] is ", buffer);
if (ispalindrome(buffer))
puts("a palindrome");
else
puts("not a palindrome");
return 0;
}
However, there is logical error. When I input anything, the output is always palindrome.
I don't know where are errors.
For example, for get "A man, a plan, a canal, Panama!", I removed(disregarded) 'space' and 'punctuation marks', changed capital letters to small letter. (amanaplanacanalpanama) However, when i input like 'abcdef', I got also "[abcedf] is a palindrome."
I am not here judging the efficiency of your code (see the comments for some suggestions on how to improve your algorithm), but there are a number of 'simple' errors in your code that can be fixed.
The code below is a minimal modification of your own that works, with triple-slash (///) comments where I've made changes:
#include <stdio.h>
#include <ctype.h>
int ispalindrome(char* str)
{
int i, j, k = 0; /// We need "k" to count the length of our PROCESSED string
char testBuffer[1024]; /// We need an ACTUAL BUFFER for our processed string ...
char* pal = &testBuffer[0];/// ... and start with "pal" pointing to the first element!
for (i = 0; *str != 0; i++) {
if (ispunct(*str) || isspace(*str)) { ///if *str is punctuation mark OR A SPACE, do not anything.
str++;
}
else if (isupper(*str)) { //if *str is a capital letter, change it to small letter and *pal=*str
*pal = tolower(*str);
str++;
pal++;
++k; /// Increase length of test string!
}
else { ///if *str is a small letter or a digit, *pal=*str
*pal = *str;
str++;
pal++;
++k; /// Increase length of test string!
}/// I think you forgot to include the { and }!
}
/// You left 'pal' pointing to a yet-to-be-filled character ...
pal = &testBuffer[0];/// We MUST reset "pal" to point at the BEGINNING of testBuffer!
/// "k" is the length of our processed string … NOT "i"...
for (j = 0; pal[j] == pal[k - (j + 1)] && j <= k / 2; j++) //compare pal
;
if (j > k / 2) return 1;/// Again, use "k" not "i"
return 0;
}
int main(void)
{
char buffer[1024];
// int i; /// This is never used!
// gets(buffer); /// This is an obsolete (and dangerous) function...
fgets(buffer, 1024, stdin); /// Use this instead!
printf("[%s] is ", buffer);
if (ispalindrome(buffer))
puts("a palindrome");
else
puts("not a palindrome");
return 0;
}
When you understand the reasons behind the required changes, I may then post an 'appendix' that suggests some efficiency/style improvements.
Appendix:
OK, you state in your question that you are studying pointers in C, so here's a version of your code that may give you some idea of what can and cannot be done with character string pointers. I don't claim that it is the most efficient method possible, but will hopefully help you understand the underlying concepts:
int ispalindrome(const char* str) // Not required, but use of "const" is good to prevent accidentally changing
{ // something you shouldn't be changing (see next comment)
char* testBuffer = malloc(strlen(str)); // It maybe tempting to just re-use the given "str" buffer, but doing
if (testBuffer == NULL) return 0; // so would spoil any later display by the calling program!
// We should always check the return value of "malloc" but what do do in case of error is up to you
char* lastTest = testBuffer; // Keeps track of the index to the last character in test string
while (*str) // We could use while (*str != 0) but this does the same and is more succinct!
{
if (isalnum(*str)) // Rather than check for what we don't what, check for what we do want ...
{ // The "isalnum" function is TRUE for any letter or digit, FALSE otherwise
*lastTest++ = tolower(*str); // The "tolower" funtion will leave uppercase and digits unchanged
// Note: The "++" (post-increment) operator will increase "lastTest" AFTER we have assigned to it
}
++str; // Move to the next character in our input string
}
// At this point, "lastTest" points to one beyond the last character in our processed string. We can loop using
// this as a pointer to our 'right-hand' compare (after we decrement it) and a 'local' pointer to our 'left-hand'
// compare (initialised to the first element of "testBuffer") ...
char* lhCompare;
for (lhCompare = testBuffer; (--lastTest > lhCompare) && (*lastTest == *lhCompare); ++lhCompare)
;
// Note: It is perfectly 'correct' and legal in C to compare two pointers to the same type! So, rather than
// keeping track of string lengths and indexes, we can just compare the 'lastTest' and 'rhCompare' addresses
// to see if we have reached/crossed the middle of the test string.
free(testBuffer); // Always call "free" to release memory allocated by "malloc" when you're done with it
// In the test below, we can still use the pointer values, even though the memory they refer to is no longer
// valid. But we CANNOT dereference them, with something like: char a = *lhCompare to get the 'middle' letter!
return (lhCompare >= lastTest); // Comparison will be TRUE (= 1) if our letters all match, FALSE (0) otherwise
}
As before, please feel free to ask for any further clarification and/or explanation of the code or concepts used.