Check a string to see if it is a palindrome

Question

I have written two code pieces to check if a given string is a palindrome:

public static bool FirstMethod(string str)
{
    int i = 0;
    int j = str.Length - 1;

    while (i<j)
    {
        if (str[i] != str[j])
            return false;

        i++;
        j--;
    }

    return true;
}

public static bool SecondMethod(string myString)
{
    string first = myString.Substring(0, myString.Length / 2);
    char[] arr = myString.ToCharArray();
    Array.Reverse(arr);
    string temp = new string(arr);
    string second = temp.Substring(0, temp.Length / 2);
    return first.Equals(second);
}

Please tell me which one is better.

Is there a better method than this?

Answer 1

I would prefer the first one for a few reasons.

1. It's more readable and easily understandable in my opinion.
2. There's only one string, so less overhead. (Which is negligible unless this is being called a lot.)
3. There's only one array. Reverse creates a second array.

That being said, an actual review is in order.

• Methods that return a boolean value should have names of the form IsSomething or HasSomething. A great name would be IsPalindrome.
• Neither method accepts anything but a string. Numbers can be palindromic too. Wouldn't it be a nice addition if we could pass an integer to it as well? (Without explicitly calling .ToString() before passing it in that is.)
• I like that you're only checking for i < j instead of i <= j. It saves a useless iteration when there are an odd number of characters.
• I also like the while loop. It's a lot cleaner than the for loop suggested by another answer.
• I won't speak much on the second method, because I think @elios provided a nice implementation of it.

Answer 2

Here is the most common solution for this problem:

public static bool IsPalindrome(string s)
{
    char[] array = s.ToCharArray();
    Array.Reverse(array);
    string backwards = new string(array);

    return s == backwards;
}

and if you are using Linq:

public static bool IsPalindrome(string s)
{
    return s == new string(s.Reverse().ToArray());
}

but if you are planning to execute this code thousands of times (which I doubt), your first method would be the fastest.

Answer 3

I agree with @ckuhn203 in that the first looks cleaner.

I'm not too familiar with C# and its style, but in C I would definitely prefer using a for loop:

public static bool FirstMethod(string str)
{
    for (int i = 0, j = str.Length - 1; i < j; i++, j--) {
        if (str[i] != str[j]) {
            return false;
        }
    }
    return true;
}

Answer 4

Consider using LINQ:

static bool IsPalindrome(string s)
{
    var characterPairs = s.Zip(s.Reverse(), (l, r) => l == r);
    return characterPairs.Take(s.Length / 2).All(isMatch => isMatch);
}

Answer 5

The first one is probably a bit faster, the second could be cleaned up a bit, but is easier to read. Unless you know you have a performance problem, prefer the second one.

For the second option, you should extract two sub-strings from the original input string called "firstHalf" and "secondHalf" corresponding to the first and second half of the input string respectively. Then reverse "secondHalf" and compare.

Answer 6

Another way to write this using LINQ, though I'm not sure it's actually clearer:

Enumerable.Range(0, str.Length / 2).All(i => str[i] == str[str.Length - i - 1])

Answer 7

My computer science teacher would turn over if at least one person didn't respond with an example of recursion. I'm not a C# guy myself, but if you wanted to nerd out I think that would work a little something like this:

public static bool IsPalindrome(string str)
{
    int len = str.length - 1;

    return str[0] == str[len] && (
           len <= 1 || IsPalindrome( str.Substring(1,len) )
           );
}

As long as you are chopping up new strings you could potentially write this so that long strings could be distributed across multiple cores. That's a 1/number_of_cores improvement!

Answer 8

I would add that you could make your method an extension method of string. It just emphasises that being a palindrome is a property of a string representation of some sort.

For ease of understanding, the second method stands out. The index based version works, but most people would scratch their heads if they saw it without a hint.

Also it's a premature optimisation to cut the string in half, even though of course it is logical. Much more clear to reverse and compare. If somehow it really is a bottleneck, then cut it in half.

Answer 9

Another shameless linq answer that doesn't require reversing or creating a new enumeration and only goes as far as it needs to:

public static bool IsPalindrome(this string str)
{
    int len = str.Length;
    return str.Take(len / 2).All(o => o == str[--len]);
}