Leo Chashchin Leo Chashchin - 3 years ago 145
C# Question

Unorthodox approach to checking if a string is a palindrome

I'm attempting to create an unorthodox approach to working out if a string is palindrome or not. I am aware of the current .Reverse() method that you can use but this is the way that I am trying to do it:

The user will enter the string and then the program will check the first and last letters of the string. If they are the same, it will keep checking. If they are not (at the start or at any particular point) then the program will declare that it is not a palindrome. Once the program stops checking and sees that no letters are not the same, it will declare that it is a palindrome.

Here is my current code below:

using System;
using System.Collections.Generic;

namespace tasks
{
class Program
{
public static void Main(string[] args)
{
Console.WriteLine("Enter a message and I will check if it is a palindrome: ");
string message1 = Convert.ToString(Console.ReadLine().ToLower());
char[] message = message1.ToCharArray();
int i = 0;
int j = message.Length - 1;
if (message[i] == message[j])
{
do
{
i++;
j--;
if (message[i] == message[j])
Console.Write("This is a palindrome");
else if (message[i] != message[j])
Console.Write("This is not a palindrome");
break;
} while (message[i] == message[j]);
}
else if (message[i] != message[j])
{
Console.WriteLine("This is not a palindrome");
}
}
}


(Sorry about the indentation).

If I was to type in 'haegah', the program would say that this is a palindrome. When it is clearly not. My question is, what is causing my program to do this and how can I fix it?

Answer Source

You've neglected to wrap { and } around Console.Write("This is not a palindrome"); break;. When you do this, only the first expression becomes conditional. The break; isn't encapsulated inside of the else branch, and will execute unconditionally thus terminating your loop in the first iteration.

This explains what I think is causing one of your problems, and how you can fix it. However, there are other problems.

By incrementing&decrementing your counters before you compare the characters, you're skipping two characters (the ones at the start and end). You can perform the incrementation & decrementation simultaneously in your loop condition with a comparison like: message[i++] == message[j--]...

Even if you fix these, your loop won't terminate sanely; the two variables will overlap, and then you'll continue until one is negative and the other is out of bounds (well, technically, they're both out of bounds)... In addition to your loop executing while message[i] == message[j], it should also only execute while i < j.

But wait, there's another problem! What do you suppose will happen when the string is an empty string? You need to guard against that condition before your loop, or use a while (...) { ... } loop.

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download