how to refactor code to return IsPalindrome

Question

using a mock c# question on https://www.testdome.com/t

I have coded

using System;

public class Palindrome
{
   public static bool IsPalindrome(string word)
        {
            string testWord = word;

            string first = word[0].ToString();
            string last = word[word.Length - 1].ToString();

            bool isPal = false;

            while (testWord.Length > 1)
            {
                Console.WriteLine(testWord);

                if (first.ToLower() == last.ToLower())
                {
                    testWord = testWord.Remove(0,1);
                    testWord = testWord.Remove(testWord.Length - 1);

                    isPal = true;
                }

            }

            return isPal;
        }

    public static void Main(string[] args)
    {
        Console.WriteLine(Palindrome.IsPalindrome("Deleveled"));
    }
}

this code is working but it is failing me on lowercase words: time limit exceeded various words: time limit exceeded.

What changes could I make to refactor the code to work faster?

Just off the top of my head (and maybe doesn't fit here) but perhaps consider just [reversing the string](https://stackoverflow.com/questions/228038/best-way-to-reverse-a-string) and testing that the `ToLower(result)` matches the `ToLower(input)`. Like 5 lines of code and no loops. — JNevill, Apr 08 '19 at 17:14
You loop forever if the first and last characters aren't equal.. add `else return false;` on that `if`. — Blorgbeard, Apr 08 '19 at 17:14
Thanks everyone for the help. I went with @JNevill If you want the points. add the answer and I will tick it — John, Apr 08 '19 at 17:24
@John Note that `ToUpperInvariant` [should be better](https://stackoverflow.com/q/2801508/11683). ([Or not](https://stackoverflow.com/a/35713225/11683).) But ultimately, if you are okay with/allowed to allocate another reversed string, then you should be comparing it to the original with `String.Equals (s1, s2, StringComparison.OrdinalIgnoreCase)` instead of converting both strings to lower or upper case. — GSerg, Apr 08 '19 at 17:30

Prasad Telkikar · Answer 1 · 2019-04-08T17:40:01.800

No need to write multiple line of code, you can check Palindrome in one line,

Magic of Linq,

bool isPalindrome = str.SequenceEqual(str.Reverse());

If you want to ignore case then convert original string and reverse string to lower case and then check its sequence

    string str = "Madam";
    var strReverse = str.ToLower().Reverse();
    var isPalindrome = str.ToLower().SequenceEqual(strReverse);

Basically, Palindrome check is a check where actual string is equal to its reverse. When we check original string to reverse string that time we need not to travel till the end. We just need to check first letter with it's last and so on...

Here is non-Linq Palindrome check,

   public static bool IsPalindrome(string word)
    {
        string testWord = word;
        for(int i = 0; i < word.Length/2; i++)
        {
            if(char.ToLower(testWord[i]) != char.ToLower(testWord[testWord.Length - i - 1]))
               return false;
        }
        return true;
    }

POC : . net fiddle

thanks for the reply but it wouldnt allow linq in this mock – John Apr 08 '19 at 17:25 — John, Apr 08 '19 at 17:25

score 0 · Answer 2 · answered Apr 08 '19 at 17:14

0

One thing you can do is immediately return if you find a non-match.

answered Apr 08 '19 at 17:14

Ben Krueger

1,476
1
14
20

how to refactor code to return IsPalindrome

2 Answers2