This is the question:
" Write a program that extracts from a text all words which are palindromes, such as ABBA", "lamal", "exe". "
And this is my code:
public static List<string> FindPalindromes()
{
string text = String.Empty;
Console.Write("Enter the text:\n\t");
text = Console.ReadLine();
List<string> answer = new List<string>();
string[] words = text.Split('.', ',', ' ', ':', '/', '\\', '"', ';');
foreach(string word in words.Where(
(string x) =>
{
if(String.Equals(x, x.Reverse()))
return true;
else
return false;
}
))
answer.Add(word);
return answer;
}
Now I think that it would more tidy if I separated the logic in the where method into a separate method that returns a Boolean and checks if a single word is a palindrome. But I want to try using a lambda.
Anyhow, this code does not return anything. I suspect the problem is in the if conditional.
x.Reverse()
is calling Enumerable.Reverse()
, which will return you an IEnumerable<char>
- not a string. That's why Equals
is never returning true
. Here's an alternative:
char[] chars = x.ToCharArray();
Array.Reverse(chars);
return x == new string(chars);
Or you could just call string.Join
or string.Concat
on the reversed sequence of characters - horribly inefficient, but it'll get the job done in a single expression, allowing you to replace everything from the foreach
onwards by:
return words.Where(x => x == string.Concat(x.Reverse())
.ToList();
Much cleaner :) Any time you find yourself repeatedly adding to a list, consider using a query and ToList()
. You'd already got the filtering part, you just needed to use ToList()
to get rid of the foreach
loop.
Likewise, any time you find yourself with:
if (condition)
return true;
else
return false;
... strongly consider refactoring to:
return condition;
See more on this question at Stackoverflow