0
namespace Palindrome
{
    class Program
    {
        public static bool IsPalindrome(string s)
        {
            int min = 0;
            int max = s.Length - 1;
            while (true)
            {
                if (min > max) // True if we've compared up to, and then gone passed the middle of the string.
                    return true;

                if (char.ToLower(s[min++]) != char.ToLower(s[max]))
                    return false;
            }
        }

        static void Main(string[] args)
        {
            string [] words = {
                                "civic",
                                "deified",
                                // ...
                                "stats",
                                "tenet",
                             };

            foreach (string value in words)
            {
                Console.WriteLine("{0} = {1}", value, IsPalindrome(value));
            }

                   Console.WriteLine("\nPress any key to continue...");
            Console.ReadKey(true);        }
    }
}

The program checks to see if the words in the words array are Palindromes (words spelled the same forwards as they are backwards).

The foreach loop in Main passes each of the words in the array to the IsPalindrome() function; which tests the word, and returns True or False accordingly.

As each word in the current array is a Palindrome, when the program is run, it should output all of the current words, followed by True. However, it gives me False. Why is that?

CodeCaster
  • 147,647
  • 23
  • 218
  • 272
boy
  • 1
  • 1
  • 3

6 Answers6

3

Seems to me like you've forgotten to decrease the value of max; i.e. the following:

if (char.ToLower(s[min++]) != char.ToLower(s[max]))

... should probably be:

if (char.ToLower(s[min++]) != char.ToLower(s[max--]))

Anyway, you should be able to clear this up pretty quickly if you run through it in debug mode, and step through your code.

Kjartan
  • 18,591
  • 15
  • 71
  • 96
2

Try

if (char.ToLower(s[min++]) != char.ToLower(s[max--]))
                return false;
Adrian Nasui
  • 1,054
  • 9
  • 10
  • 1
    sure, you should really learn to use debugging, ask your teacher to help with this. And don't forget to mark an answer! – Adrian Nasui Dec 11 '14 at 11:46
1

I think you forgot to decrement your max variable.

Now you're just comparing each letter with the last.

Rik
  • 28,507
  • 14
  • 48
  • 67
0

You already got answers about whats wrong with your code. Here's an cleaner solution option(if you dont need any performance optimizations):

public static bool IsPalindrome(string s)
{
   char[] array = s.ToCharArray();
   Array.Reverse(array);
   string backwards = new string(array);
   return s.Equals(backwards, StringComparison.OrdinalIgnoreCase);
}
Amir Popovich
  • 29,350
  • 9
  • 53
  • 99
  • What happens if the first and last text elements don't have exactly the same chars? – Jodrell Dec 11 '14 at 12:06
  • @Jodrell - What do you mean? We are talking about strings.."dad" will return true, "da" will return false.. Can you please provide a use case where this fails? – Amir Popovich Dec 11 '14 at 12:14
  • A given "text element" (The letter that is rendered on screen), can be represented my multiple combinations of Unicode Code Points and therefore different combinations of `char` values. So "text elements" can look the same, and from many perspectives be the same but be ordinally different. This answer expands http://stackoverflow.com/a/26977869/659190 – Jodrell Dec 11 '14 at 12:18
  • ok - But how does he solve this in his solution? doing str[min] == str[max] is pretty much the same. I don't think I need to support these extreme special cases..If it was so important, I believe .net would support this in the String class somehow.. – Amir Popovich Dec 11 '14 at 13:57
  • @Jodrell - From what I understand, If I would have added a StringComparer param with a default of CurrentCultureIgnoreCase then our methods would have worked pretty much the same.. – Amir Popovich Dec 11 '14 at 14:27
0

You have to decrease your max value.

For another method of testing.

var input = "abba";
var output = input.ToCharArray().Reverse().Aggregate("",(x,y) => x + y));
return input.Equals(output, StringComparison.OrdinalIgnoreCase);
woutervs
  • 1,500
  • 12
  • 28
  • What happens if the first and last text elements don't have exactly the same chars? – Jodrell Dec 11 '14 at 12:07
  • Then it's not a palindrome? Can you give me an example of what you are trying to say? – woutervs Dec 12 '14 at 08:37
  • A given "text element" (The letter that is rendered on screen), can be represented my multiple combinations of Unicode Code Points and therefore different combinations of char values. So "text elements" can look the same, and from many perspectives be the same but be ordinally different. This answer expands stackoverflow.com/a/26977869/659190 – Jodrell Dec 12 '14 at 08:49
  • Then of course the palindrome function would return false. As since the characters don't match even though they look the same. You would need to make a hashmap and a specialized equals function to work around this issue. Then again the question is: will this ever happen so do I need to invest time and money in creating such feature? – woutervs Dec 12 '14 at 13:23
  • That depends if you want an `IsPalindrome` function that works with any string, in a way that is definable or a function that is optimized for but works only for a subset of strings. – Jodrell Dec 12 '14 at 14:31
0

here you go, a better culture/case insensitive version

using System.Globalization;

bool IsPalindrome(string value, StringComparer comparer = null)
{
    if (s == null)
    {
        throw new ArgumentNullException("value");
    }

    if (comparer == null)
    {
        comparer = StringComparer.CurrentCultureIgnoreCase;
    }

    var elements = new List<string>();
    var m = StringInfo.GetTextElementEnumerator(value);
    while (m.MoveNext())
    {
        elements.Add(m.GetTextElement());
    }

    var i = 0;
    var j = elements.Count - 1;
    var limit = elements.Count / 2;
    for(; i <= limit; i++, j--)
    {
        if (!comparer.Equals(elements[i], elements[j]))
        {
            return false;
        }
    }

    return true;
}
Jodrell
  • 34,946
  • 5
  • 87
  • 124