HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Remove unwanted characters from a string

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
unwantedremovecharactersfromstring

Problem

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?

Solution

-
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 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(allowedCharacters));

}  
private string RemoveUnwantedChar(string input, HashSet 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.

Code Snippets

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();
}
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();
}

Context

StackExchange Code Review Q#109513, answer score: 14

Revisions (0)

No revisions yet.