Skip to main content
2 of 5
clean
Caridorc
  • 28.1k
  • 7
  • 55
  • 138

Use a for loop where fit

public static string MergeMultipleLinesIntoOneLine(int numberOfLines)
{
    
    var stringResult = new StringBuilder();

    for (var line = 0; line < numberOfLines; line++)
    {
        Console.WriteLine("Please enter phrase #{0}: ", line);
        stringResult.Append(Console.ReadLine() + " ");
    }

    return stringResult.ToString();
}

MergeMultipleLinesIntoOneLine was very weird, when looping a fixed number of times, you should always use a for loop.

Refactor further

isPalindrome does two things:

  • It cleans the string.
  • It actually checks if it a palindrome.

These are two things, so we need two functions:

public static string clean(string s) 
{
    string regexPattern = @"(?i)[^a-z]";
    string replacement = "";
    return Regex.Replace(s, regexPattern, replacement).ToLower();
}

and:

public static bool IsPalindrome(string someString)
{
    var amendedString = clean(someString);
    string reverseAmendedString = new string(amendedString.Reverse().ToArray());
    return amendedString.Equals(reverseAmendedString);
}

Use constants

The cleaner Regex should be a class constant. Do not put types into the name of the variables.

private const string cleaner = @"(?i)[^a-z]";

And replacement is unlikely to change, just use ""

clean is now simpler:

public static string clean(string s) 
{
    return Regex.Replace(s, cleaner, "").ToLower();
}

Avoid unnecessary variables

bool isPalindrome = IsPalindrome(mergedLine);

Delete that line, just use:

Console.WriteLine(isPalindrome(mergedLine) ? "Palindrome" : "Not a palindrome");

Also isPalindrome can use less verbose names and be reduced to:

public static bool IsPalindrome(string someString)
{
    var cleaned = clean(someString);
    return cleaned.Equals(amendedString.Reverse().ToArray());
}

Seriously reducing noise.

Caridorc
  • 28.1k
  • 7
  • 55
  • 138