123

Is there a better way of doing this...

MyString.Trim().Replace("&", "and").Replace(",", "").Replace(" ", " ")
 .Replace(" ", "-").Replace("'", "").Replace("/", "").ToLower();

I've extended the string class to keep it down to one job but is there a quicker way?

public static class StringExtension
{
 public static string clean(this string s)
 {
 return s.Replace("&", "and").Replace(",", "").Replace(" ", " ")
 .Replace(" ", "-").Replace("'", "").Replace(".", "")
 .Replace("eacute;", "é").ToLower();
 }
}

Just for fun (and to stop the arguments in the comments) I've shoved a gist up benchmarking the various examples below.

https://gist.github.com/ChrisMcKee/5937656

The regex option scores terribly; the dictionary option comes up the fastest; the long winded version of the stringbuilder replace is slightly faster than the short hand.

asked Aug 24, 2009 at 9:25
6
  • 1
    Based on what you have in your benchmarks it looks like the dictionary version isn't doing all of the replacements which I suspect is what is making it faster than the StringBuilder solutions. Commented Sep 12, 2014 at 21:23
  • 1
    @toad Hi from 2009; I added a comment below in April about that glaring mistake. The gist is updated though I skipped over D. The dictionary version is still faster. Commented Sep 15, 2014 at 0:24
  • Possible duplicate of Alternative to String.Replace multiple times? Commented Mar 17, 2016 at 20:19
  • 1
    @TotZam at least check the dates before flagging things; this is from 2009 thats from 2012 Commented Jun 22, 2016 at 13:27
  • Since many answers here seem concerned with performance, I believe it should be pointed out Andrej Adamanko's answer is likely to be the fastest for many replacements; certainly faster than chaining .Replace() especially on a large input string as stated in his answer. Commented Dec 9, 2019 at 1:05

10 Answers 10

168

Quicker - no. More effective - yes, if you will use the StringBuilder class. With your implementation each operation generates a copy of a string which under circumstances may impair performance. Strings are immutable objects so each operation just returns a modified copy.

If you expect this method to be actively called on multiple Strings of significant length, it might be better to "migrate" its implementation onto the StringBuilder class. With it any modification is performed directly on that instance, so you spare unnecessary copy operations.

public static class StringExtention
{
 public static string clean(this string s)
 {
 StringBuilder sb = new StringBuilder (s);
 sb.Replace("&", "and");
 sb.Replace(",", "");
 sb.Replace(" ", " ");
 sb.Replace(" ", "-");
 sb.Replace("'", "");
 sb.Replace(".", "");
 sb.Replace("eacute;", "é");
 return sb.ToString().ToLower();
 }
}
BC2
8921 gold badge7 silver badges23 bronze badges
answered Aug 24, 2009 at 9:27
Sign up to request clarification or add additional context in comments.

6 Comments

For clarity the dictionary answer is the fastest stackoverflow.com/a/1321366/52912
In your benchmark on gist.github.com/ChrisMcKee/5937656 the dictionary test is not complete: it does not do all replacements and " " replaces " ", not " ". Not doing all replacements could be the reason, why it's fastest in the benchmark. The regex replacement is not complete, either. But most importantly your string TestData is very short. Like the accepted answer states, the string has to be of significant length for the StringBuilder to be of advantage. Could you please repeat the benchmark with strings of 10kB, 100kB and 1MB?
Its a good point; as it stands it was being used for url cleansing so testings at 100kb - 1mb would have been unrealistic. I will update the benchmark so its using the whole thing though, that was a mistake.
For best performance, loop over the characters and replace them yourself. However that can be tedious if you have more than single characters strings (find them enforces you to compare multiple characters at once, while replacing them requires allocating more memory and moving the rest of the string).
When none of the characters or strings to be replaced occur in the input string, this will be a very bad solution. In that case String.Replace would just return the original reference and be dirt cheap compared to the StringBuilder solution.
|
32

If you are simply after a pretty solution and don't need to save a few nanoseconds, how about some LINQ sugar?

var input = "test1test2test3";
var replacements = new Dictionary<string, string> { { "1", "*" }, { "2", "_" }, { "3", "&" } };
var output = replacements.Aggregate(input, (current, replacement) => current.Replace(replacement.Key, replacement.Value));
answered May 6, 2014 at 2:21

3 Comments

Similar to example C in the Gist (if you look above it the uglier linq statement is in the comment)
Interesting that you define a functional statment as "Uglier" than a procedural one.
not going to argue about it; its merely preference. As you say, linq is simply syntactic sugar; and as I said I'd already put the equivalent above the code :)
20

this will be more efficient:

public static class StringExtension
{
 public static string clean(this string s)
 {
 return new StringBuilder(s)
 .Replace("&", "and")
 .Replace(",", "")
 .Replace(" ", " ")
 .Replace(" ", "-")
 .Replace("'", "")
 .Replace(".", "")
 .Replace("eacute;", "é")
 .ToString()
 .ToLower();
 }
}
Magnus
47.2k8 gold badges88 silver badges128 bronze badges
answered Aug 24, 2009 at 9:31

2 Comments

Really hard to read. I am sure you know what it does but a Junior Dev will scratch his head at what actually goes on. I agree- I also always look for the shortes hand of writting something- But it was only for my own satisfaction. Other people were freaking out at the pile of mess.
This is actually slower. BenchmarkOverhead... 13ms StringClean-user151323... 2843ms StringClean-TheVillageIdiot... 2921ms Varies on reruns but the answer wins gist.github.com/anonymous/5937596
11

Maybe a little more readable?

 public static class StringExtension {
 private static Dictionary<string, string> _replacements = new Dictionary<string, string>();
 static StringExtension() {
 _replacements["&"] = "and";
 _replacements[","] = "";
 _replacements[" "] = " ";
 // etc...
 }
 public static string clean(this string s) {
 foreach (string to_replace in _replacements.Keys) {
 s = s.Replace(to_replace, _replacements[to_replace]);
 }
 return s;
 }
 }

Also add New In Town's suggestion about StringBuilder...

answered Aug 24, 2009 at 9:33

3 Comments

It would be more readable like this: private static Dictionary<string, string> _replacements = new Dictionary<string, string>() { {"&", "and"}, {",", ""}, {" ", " "} /* etc */ };
or of course... private static readonly Dictionary<string, string> Replacements = new Dictionary<string, string>() { { "&", "and" }, { ",", "" }, { " ", " " } /* etc */ }; public static string Clean(this string s) { return Replacements.Keys.Aggregate(s, (current, toReplace) => current.Replace(toReplace, Replacements[toReplace])); }
-1 : Using a Dictionary doesen't make any sence here. Just use a List<Tuple<string,string>>. This also changes the order of the replacings is taken AND is not as fast as e.g. s.Replace("a").Replace("b").Replace("c"). Don't use this!
11

There is one thing that may be optimized in the suggested solutions. Having many calls to Replace() makes the code to do multiple passes over the same string. With very long strings the solutions may be slow because of CPU cache capacity misses. May be one should consider replacing multiple strings in a single pass.

The essential content from that link:

static string MultipleReplace(string text, Dictionary<string, string> replacements) {
 return Regex.Replace(text,
 "(" + String.Join("|", replacements.Keys) + ")",
 delegate(Match m) { return replacements[m.Value]; });
}
 // somewhere else in code
 string temp = "Jonathan Smith is a developer";
 var adict = new Dictionary<string, string>();
 adict.Add("Jonathan", "David");
 adict.Add("Smith", "Seruyange");
 string rep = MultipleReplace(temp, adict);
mskfisher
3,4124 gold badges37 silver badges50 bronze badges
answered May 21, 2014 at 13:21

3 Comments

A lot of answers seem concerned about performance, in which case this is the best. And it's simple because it's just a documented overload of String.Replace where you return an expected value based on the match, in this example, using a dictionary to match them up. Should be simple to understand.
Added code from linked page to prevent this answer becoming useless if linked page dies
Please correct your answer so the code actually works. In your MultipleReplace method there is no variable called 'adict', change it to 'replacements and remove the .ToArray(). Indent code correctly to make it more readable. Thank you for the solution.
5

Another option using linq is

[TestMethod]
public void Test()
{
 var input = "it's worth a lot of money, if you can find a buyer.";
 var expected = "its worth a lot of money if you can find a buyer";
 var removeList = new string[] { ".", ",", "'" };
 var result = input;
 removeList.ToList().ForEach(o => result = result.Replace(o, string.Empty));
 Assert.AreEqual(expected, result);
}
Chris McKee
4,42711 gold badges52 silver badges83 bronze badges
answered Nov 9, 2017 at 11:15

3 Comments

You may declare var removeList = new List<string> { /*...*/ }; then just call removeList.ForEach( /*...*/ ); and simplify your code. Note also that it doesn't fully answer the question because all found strings are replaced with String.Empty.
Where precisely is Linq used? This wastefully converts removeList to a List, for the unnecessary goal of making it a single line. But Lamdas and Linq aren't synonymous.
Note, List.ForEach is not a LINQ thing, it's a List thing
0

I'm doing something similar, but in my case I'm doing serialization/De-serialization so I need to be able to go both directions. I find using a string[][] works nearly identically to the dictionary, including initialization, but you can go the other direction too, returning the substitutes to their original values, something that the dictionary really isn't set up to do.

Edit: You can use Dictionary<Key,List<Values>> in order to obtain same result as string[][]

radu florescu
4,36310 gold badges64 silver badges95 bronze badges
answered Sep 16, 2011 at 22:57

1 Comment

This doesn't appear to provide an answer to the question
0

Regular Expression with MatchEvaluator could also be used:

 var pattern = new Regex(@"These|words|are|placed|in|parentheses");
 var input = "The matching words in this text are being placed inside parentheses.";
 var result = pattern.Replace(input , match=> $"({match.Value})");

Note:

  • Obviously different expression (like: \b(\w*test\w*)\b) could be used for words matching.
  • I was hoping it to be more optimized to find the pattern in expression and do the replacements
  • The advantage is the ability to process the matching elements while doing the replacements
answered Sep 23, 2021 at 22:20

1 Comment

This answer would be improved by showing a better use of the match delegate than simply supplying the same value that was matched; it's a non op
0

This is essentially Paolo Tedesco's answer, but I wanted to make it re-usable.

 public class StringMultipleReplaceHelper
 {
 private readonly Dictionary<string, string> _replacements;
 public StringMultipleReplaceHelper(Dictionary<string, string> replacements)
 {
 _replacements = replacements;
 }
 public string clean(string s)
 {
 foreach (string to_replace in _replacements.Keys)
 {
 s = s.Replace(to_replace, _replacements[to_replace]);
 }
 return s;
 }
 }

One thing to note that I had to stop it being an extension, remove the static modifiers, and remove this from clean(this string s). I'm open to suggestions as to how to implement this better.

answered Feb 22, 2022 at 11:27

1 Comment

This does not work well if replacements have common prefixes.
-1
string input = "it's worth a lot of money, if you can find a buyer.";
for (dynamic i = 0, repl = new string[,] { { "'", "''" }, { "money", "$" }, { "find", "locate" } }; i < repl.Length / 2; i++) {
 input = input.Replace(repl[i, 0], repl[i, 1]);
}
answered Mar 16, 2017 at 0:23

1 Comment

You should consider adding context to your answers. Like a brief explanation of what's it doing And, if relevant, why you wrote it the way you did.

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.