patterncsharpModerate
Remove unwanted characters from a string
Viewed 0 times
unwantedremovecharactersfromstring
Problem
From what I've seen in other posts, if I actually know the unwanted characters, then I can do
Characters that I want:
How can I tidy this code to be neater and more readable?
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
-
using string concatenation in a loop is mostly a bad idea. Use a
-
omitting braces
Implementing the mentioned points will lead to
@Caricorc made a good suggestion in the comments
In my opinion
So by passing the
If performance is an issue, you could also pass a
you can reuse it somewhere else.
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.