Remove unwanted characters from a string

Question

From what I've seen in other posts, if I actually know the unwanted characters, then I can do string.replace(). But in my case, any characters can appear in my input string, and I only want to keep the characters that I want (without messing up the order of course).

private string RemoveUnwantedChar(string input)
{
    string correctString = "";

    for (int i = 0; i < input.Length; i++)
    {
        if (char.IsDigit(input[i]) || input[i] == '.' || input[i] == '-' || input[i] == 'n'
                || input[i] == 'u' || input[i] == 'm' || input[i] == 'k' || input[i] == 'M'
                || input[i] == 'G' || input[i] == 'H' || input[i] == 'z' || input[i] == 'V'
                || input[i] == 's' || input[i] == '%')
            correctString += input[i];
    }
    return correctString;
}

Characters that I want:

• 0123456789
• numkMGHzVs%-

How can I tidy this code to be neater and more readable?

Answer 1

• By having a const string which contains all of your wanted chars, you could do either a simple call to Contains() or check if IndexOf() will return a value > -1.

• using string concatenation in a loop is mostly a bad idea. Use a StringBuilder instead.

• omitting braces {} although they are optional for single lined if statements is a bad idea because it makes your code error prone.

Implementing the mentioned points will lead to

private const string allowedCharacters = "numkMGHzVs%-.";
private string RemoveUnwantedChar(string input)
{

    StringBuilder builder = new StringBuilder(input.Length);

    for (int i = 0; i < input.Length; i++)
    {
        if (char.IsDigit(input[i]) || allowedCharacters.Contains(input[i]))
        {
            builder.Append(input[i]);
        }
    }
    return builder.ToString();
}  

@Caricorc made a good suggestion in the comments

In my opinion allowedCharacters should be an argument to the function to allow reusability.

So by passing the allowedCharacters as an optional parameter with an additional check with IsNullOrEmpty().

If performance is an issue, you could also pass a HashSet<char> to the method or have an overloaded method like so

private string RemoveUnwantedChar(string input, string allowedCharacters = "0123456789numkMGHzVs%-.")
{

    if (string.IsNullOrEmpty(allowedCharacters)) { return input; }

    return RemoveUnwantedChar(input, new HashSet<char>(allowedCharacters));

}  
private string RemoveUnwantedChar(string input, HashSet<char> allowedCharacters)
{
    if (allowedCharacters.Count == 0) { return input; }

    StringBuilder builder = new StringBuilder(input.Length);

    for (int i = 0; i < input.Length; i++)
    {
        if (allowedCharacters.Contains(input[i]))
        {
            builder.Append(input[i]);
        }
    }
    return builder.ToString();
}  

you can reuse it somewhere else.

Answer 2

Whenever I see filtering, I tend to think LINQ.

private string RemoveUnwantedCharacters(string input, IEnumerable<char> allowedCharacters)
{
    var filtered = input.ToCharArray()
        .Where(c => allowedCharacters.Contains(c))
        .ToArray();

    return new String(filtered);
}

You can call it like this:

string filteredString = RemoveUnwantedCharacters(inputString, "0123456789numkMGHzVs%-.");

• Code is shorter
• The intent is clear - it basically reads as "filtered is input where allowed characters contains this character", which is pretty self-explanatory
• Allowed characters is a parameter, so you can reuse the method in various places. If you're using the same set of allowed characters in a lot of places, stick them in some sort of settings store.

Answer 3

Same answer as on SO

Whenever you have to search for literals. Regex is the way to go.

public string RemoveUnwantedChar(string input) {
    StringBuilder stringBuilder = new StringBuilder();

        foreach (var match in Regex.Matches(input, "[0-9numkMGHzVs%\\-.]")) {
            stringBuilder.Append(match.ToString());
        }

        return stringBuilder.ToString();
}

• Code is shorter
• Very easy to expand
• Easy to read
• Easy to follow the Code

Second Solution. A nice OneLiner as Taemyr suggested:

public string RemoveUnwantedChar(string input) {
        return Regex.Replace(input, "[^0-9numkMGHzVs%\\-.]", "");
    }

// Edit from String concatenation to StringBuilder implementation for better Performace especially for large inputs

// Edit2 Escaped the Dash for more Info: https://stackoverflow.com/questions/9589074/regex-should-hyphens-be-escaped