Palindrome checker working but ...

[sean_cantab]

Admittedly not the best code for checking whether a string is a palindrome but it's throwing up a side problem nevertheless. While it correctly identifies and reports palindromic strings, if the input string is not a palindrome then the code does not return the printf "The string is not a palindrome". Instead it does nothing beyond reporting the strlen. I've tried using both boolean (match) and counter (sum) as described in code below. Any suggestions would be much appreciated. Thanks

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




int main()


{
	char str[100];
	int len, i;
	int j = 0;
//	int sum = 0; 	
	int match; 
	
	/* Inputs string and gets string length */
	
	printf("Enter the string: \n");
	gets(str);
	
	len = strlen(str);
	
	printf("Length of the string is %d\n", len);
	
	/* For checking whether an even string is palindromic */
	
	if (len%2==0)
	
	{
		for (i = len - 1; i>= len/2; i--)
		
		{match = 1; 
			if(str[j] != str[i])
				
				{
					//sum  = sum + 1; 
					//j++; 
					
					match = 0; 
					break;
					j++;  
				}
				
				if (match == 1)
				
				{
				printf("The string is a palindrome\n"); 
				}
				
				else
				
				printf("The string is not a palindrome\n"); 
				
		}
				
	}
	/* For checking whether odd string is palindromic*/		
	else if (len%2 !=0)
	
	{	
		for (i = len -1; i > len/2; i--)
		{match = 1; 
		{
			if(str[i] != str[j])
				
				{
				//	sum = sum + 1; 
				//	j++; 
				
				match = 0; 
				break;
				j++; 
				}
				
				if (match == 1)
				{
					printf("The string is a palindrome"); 
				}
				else
				{
					printf("The string is not a palindrome\n");
				}
				
				
		}
	}


}
}

[Matticus]

You really should learn how to consistently format and indent your code.

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

int main()
{
    char str[100];
    int len, i;
    int j = 0;
//  int sum = 0;
    int match;

    /* Inputs string and gets string length */

    printf("Enter the string: \n");
    gets(str);

    len = strlen(str);

    printf("Length of the string is %d\n", len);

    /* For checking whether an even string is palindromic */
    if (len%2==0)
    {
        for (i = len - 1; i>= len/2; i--)
        {
            match = 1;

            if(str[j] != str[i])
            {
                //sum  = sum + 1;
                //j++;

                match = 0;
                break;
                j++;
            }

            if (match == 1)
            {
                printf("The string is a palindrome\n");
            }
            else
                printf("The string is not a palindrome\n");
        }
    }
    /* For checking whether odd string is palindromic*/
    else if (len%2 !=0)
    {
        for (i = len -1; i > len/2; i--)
        {
            match = 1;

            {
                if(str[i] != str[j])
                {
                    //  sum = sum + 1;
                    //  j++;

                    match = 0;
                    break;
                    j++;
                }

                if (match == 1)
                {
                    printf("The string is a palindrome");
                }
                else
                {
                    printf("The string is not a palindrome\n");
                }
            }
        }
    }
}

Notice how the code is indented one level beneath each opening brace, and back one level at the closing brace. This also makes it clear you have an unnecessary block (lines 53 and 72) that serves no purpose.

gets(str);

Do not use "gets()"!
See here for a safer alternative: http://faq.cprogramming.com/cgi-bin/...&id=1043284385

match = 0;
break;
j++;

Note that j++ is never reached, because you break out of the loop beforehand.

Also notice that, if the characters you're comparing are not equal (e.g. the string failed the palindrome test), you break out of the loop - meaning you never reach the subsequent code that prints the "The string is not a palindrome" message.

You also don't need two separate tests depending on whether the string has an odd or even number of digits. Perhaps play around with the palindrome analysis on paper to see if you can find a way to do a single test.

[sean_cantab]

Thanks, I take on board your comments re format and indent.

About j++ never being reached upon break: if match = 0, why would I still need to increase j by 1 to check the next character? We already know the string is not palindromic. And in this case, as the break kicks in and since match != 1, should the program not then take us to line 42 (in even case) or line 68 (in odd case) and print the not palindrome statement therein?

[Matticus]

Note that "break" takes you out of the loop where it resides. You seem to be under the impression that it will break you out of the "if", which is not the case. So the "break" on line 34 will take you to line 45 (the end of the first "if" block).

I just stepped through your code with a debugger (I suggest you do the same), and it does not actually work. Look carefully at the first "for" loop in the even test. Assume you have an input of "123321".

You set "match" equal to one, do a character comparison (which is false, so that code is skipped), then print the "is a palindrome" message the first time through. Then "i" is updated, but "j" never is, since the character comparison was false the first time through. On the second iteration, the character comparison fails (because "j" was not updated the previous round), the code reaches the "break", and the program finished. Thus, it appears your program ran successfully for the test, but it did not.

I strongly suggest you plan your logic out better, by coming up with an algorithm by hand, before attempting to write it in code.

[Matticus]

Okay, I'm going to give you a quick lesson in code development. I trust that you will read, try to understand, and apply this to your programs going forward - and not just take the solution and call it a day.

First, figure out in words exactly what you need to do in order to solve the problem. Basically, you need to compare the outer-most characters. If they are not the same, you do not have a palindrome, so you print a message saying so and stop checking. Otherwise, you compare the second and second-to-last characters. If they are not the same, you do not have a palindrome, and so on. If the loop ends, and the test has not failed, you know that you do have a palindrome.

Notice how you can't know you have a palindrome until after all the characters have been tested (except, obviously, for the middle character in a string with an odd number of characters).

Now let's break this down into a series of steps:

Set variable "i" equal to zero
Set variable "j" equal to the end of the string
Compare the characters at indices "i" and "j"
If they are not the same

Print "not a palindrome"
Stop the test

Increment "i"
Decrement "j"
If "i" is less than "j", we are not yet at the middle, so repeat the test

After the test, if the test has not failed

Print "is a palindrome"

As you can see, we need to determine whether or not the test has failed in order to know if we should print the "is a palindrome" message. This can easily be done by checking the values of "i" and "j", but for clarity, let's use a flag instead. We initialize the flag to "success" before the test. If the test fails, we clear the flag. Then after the test, if the flag is still set, we know to print the "is a palindrome" message. Let's update our list of steps with this addition:

Set variable "i" equal to zero
Set variable "j" equal to the end of the string
Set variable "isPalindrome" equal to 1
Compare the characters at indices "i" and "j"
If they are not the same

Print "not a palindrome"
Set "isPalindrome" to zero
Stop the test

Increment "i"
Decrement "j"
If "i" is less than "j", we are not yet at the middle, so repeat the test

After the test, if "isPalindrome" flag is set

Print "is a palindrome"

Now we have pseudocode that describes the solution. We can read through it and verify that these steps will solve our problem.

The next step is to convert this into code. This translation is actually pretty easy, since generally one line of pseudocode corresponds to roughly one (or just a few) lines of code. Note that the "loop" logic could be fleshed out a little bit more in the pseudocode, but this example is simple enough where that might not be necessary.

I'll leave the rest up to you, but I hope this illustrates how a little thought and planning can make it easier to write good, working code.

[fredlo2010]

I just have to say that I love this forum. The community here, not just teach you how to fix code but how to become a better developer.

Cheers to that!!!

PS: I made my own version of the code and it's a mirror to the pseudocode suggested

Thanks

[sean_cantab]

Matticus, I am extremely grateful to you for your suggestions and this has helped my (limited!) understanding of programming. Many thanks. I think I have been able to work out the palindrome code now and might post it later on. For now, if I may, I'd like to revert back to this earlier post of yours to seek a couple of clarifications. You've mentioned:

Note that "break" takes you out of the loop where it resides. You seem to be under the impression that it will break you out of the "if", which is not the case. So the "break" on line 34 will take you to line 45 (the end of the first "if" block).

- here doesn't "break" reside in the loop lines 29-36, so that if we break, it should take us to line 38, the start of the next loop? Or did you imply that break takes us out of the match loop, which would mean line 45?

Look carefully at the first "for" loop in the even test. Assume you have an input of "123321".
You set "match" equal to one, do a character comparison (which is false, so that code is skipped), then print the "is a palindrome" message the first time through. Then "i" is updated, but "j" never is, since the character comparison was false the first time through. On the second iteration, the character comparison fails (because "j" was not updated the previous round), the code reaches the "break", and the program finished.

- why would the character comparison be false the first time through? If not false, then both 'i' and 'j' should be updated and the character comparison on the second iteration should also be true and so on since "123321" is palindromic?

-Later on, in the pseudocode posting, you've suggested printing "not a palindrome" and then turning off the flag and breaking the test. This works fine for the palindrome case since it's effectively a single loop but where we have nested loops, wouldn't it print "not a palindrome" multiple times and in that case would it make more sense to print the alternative statement under else if at the end when we check if match ==1?

Finally, once again, many thanks for your help which is truly appreciated. Regards

[Matticus]

- here doesn't "break" reside in the loop lines 29-36, so that if we break, it should take us to line 38, the start of the next loop? Or did you imply that break takes us out of the match loop, which would mean line 45?

As I mentioned, "if" is not a loop. The loop where the break is contained is the "for" loop starting on line 24. When that "break" is encountered, execution jumps to the end of the "for" loop (the closing brace, on line 44).

In this image, the span of the "for" loop is shown in red, and the jump from the "break" is shown in blue.

- why would the character comparison be false the first time through? If not false, then both 'i' and 'j' should be updated and the character comparison on the second iteration should also be true and so on since "123321" is palindromic?

In the given example, the character comparison would be true the first time around. But "j" is only incremented (assuming you get rid of the break) if the character comparison is false. So the second time through, you're not comparing the right indices ("i" has been updated, but "j" has not).

-Later on, in the pseudocode posting, you've suggested printing "not a palindrome" and then turning off the flag and breaking the test. This works fine for the palindrome case since it's effectively a single loop but where we have nested loops, wouldn't it print "not a palindrome" multiple times and in that case would it make more sense to print the alternative statement under else if at the end when we check if match ==1?

The algorithm I described doesn't rely on nested loops, so I'm not sure your question is valid. Perhaps you could explain where the nested loop part of your question came from?

Finally, once again, many thanks for your help which is truly appreciated. Regards

You're welcome!

[sean_cantab]

Matticus, many thanks for your patient explanations. It has certainly helped me a lot! I see what you mean about the span of the loop and the increments in j. My query about nested loops was based on something else I am working on (checking if a matrix is upper triangular) but I'll try and digest your information more thoroughly first and seek to apply it to that case. As mentioned earlier, here's the palindrome code based upon your suggestions (sorry, the gets() still remains, will try and 'get' rid of it shortly!).

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




int main()


{
	char str[100];
	int len, i;
	int j = 0;
	int match; 
	
	/* Inputs string and gets string length */
	
	printf("Enter the string: \n");
	gets(str);
	len = strlen(str);
	printf("Length of the string is %d\n", len);
	
	/* For checking whether an even string is palindromic */
	
	if (len%2==0)
	{
		for (i = len - 1; i>= len/2; i--)
		{match = 1; 
			if(str[j] != str[i])
			{
				printf("It is not a palindrome\n");
				match = 0; 
				break;
			}
				j++;  
		}
				if (match == 1)
			{
			printf("The string is a palindrome\n"); 
			}
	}
	/* For checking whether odd string is palindromic*/		
	else if (len%2 !=0)
	{	
		for (i = len -1; i > len/2; i--)
		{match = 1; 
			if(str[i] != str[j])
			{
				printf("The string is not a palindrome\n");
				match = 0; 
				break;
			}
				j++; 
		}
		if (match == 1)
			{
				printf("The string is a palindrome"); 
			}
	}
}

[algorism]

Matticus [...] here's the palindrome code based upon your suggestions

He also suggested that you rethink your algorithm and get rid of the unnecessary division into even and odd string lengths.

[sean_cantab]

Please see the OP which starts off with: "Admittedly not the best code for checking whether a string is a palindrome ..."

So, yes, I was aware of the alternative from the start but pursued this line which gave rise to some positive learning outcomes for me and at least one other person as seen by his/her comments on this thread.

But thanks for your observation nonetheless. Regards

[Matticus]

Please see the OP which starts off with: "Admittedly not the best code for checking whether a string is a palindrome ..."

Perhaps, as an exercise, you should start up a new project and attempt this program again, using only the given pseudocode as a reference. At the very least, it would be good practice.

[fredlo2010]

Sometimes when I am stuck in a function or an algorithm ; I just scratch everything and start from zero, it helps a lot.