Out of curiosity - is there a better performing approach to do this string replace?

Question

I have the below method where I need to check for some certain strings which could be in any case and then remove them. Just wondered if there was a better performing way?

private void MyMethod(string Filter)
{
   //need to remove <Filter> and </Filter> case in-sensitive
   var result = Filter.ToLower().Replace("<filter>","");
   result = Filter.ToLower().Replace("</filter>,"");

  ...........................
}

`Regex.Replace(Filter, "?filter>", string.empty, RegexOptions.IgnoreCase); ` If you are interested in the performance of various methods, there are plenty of discussions about it here on SO. — Marcel Gheorghita, Feb 08 '11 at 12:24
isnt it going to change the case of whats in between the tags. or it does not matter to you. — Mubashir Khan, Feb 08 '11 at 12:24
`ToUpper` is a better option than `ToLower`. String comparison and replacement has been optimized for the former. From the developer's perspective, there's no difference so it's a simple fix. — Cody Gray - on strike, Feb 08 '11 at 12:30
@Cody Gray - Do you have any link that verifies this? I would like to learn why that makes any difference. — Øyvind Bråthen, Feb 08 '11 at 12:33
@Øyvind: Not really. I remember reading it in the Framework Design Guidelines, and it's never mattered enough to me to take the time to verify it myself. It *does* work properly with foreign characters, though, when `ToLower` might produce unexpected results. I think that's what was meant by the recommendation. — Cody Gray - on strike, Feb 08 '11 at 12:34
@Øyvind: Actually, it turns out that it's `ToUpperInvariant`. Which makes sense; that's what you should be using anyway. And here you go with some links: http://stackoverflow.com/questions/9033/hidden-features-of-c#12137 and http://msdn.microsoft.com/en-us/library/ms973919.aspx, specifically "Recommendations for String Use" — Cody Gray - on strike, Feb 08 '11 at 12:37
@Cody Gray - Thanks a lot. It seems that it's to prevent unexpected results, and not performance, and that gives a lot more sense (even if I would never have guessed on my own). You learn something new every day ;) — Øyvind Bråthen, Feb 08 '11 at 12:51
You can also stack .replace(). You can write the code as string.replace().replace()... Should save a few extra cycles there by not having to run the equation again. — guildsbounty, Feb 08 '11 at 14:25

score 4 · Answer 1 · edited May 23 '17 at 11:47

4

Check this answer: Is there an alternative to string.Replace that is case-insensitive?

You might want to do a comparison with a performance check. Profile this with a profiler. It is the only way to really know, what is faster.

But honestly: Does performance really matter? How often are you doing this? I can't really see you doing this so often, that performance will become an issue...

You could try Regex.Replace, with a case insensitive replace. This is not faster. But it is case insensitive.

edited May 23 '17 at 11:47

Community

1
1

answered Feb 08 '11 at 12:24

Daren Thomas

67,947
40
154
200

Why do you think a regex is going to be faster? And if not, why do you recommend trying that instead? – Cody Gray - on strike Feb 08 '11 at 12:28
I do not think he asserted that `Regex.Replace` is faster, quite the contrary. – Marcel Gheorghita Feb 08 '11 at 12:31
No, I don't think Regex.Replace is faster. But it does do a case insensitive replace. I will update my answer. – Daren Thomas Feb 08 '11 at 14:13
I do think RegEx will be faster under certain conditions. Not sure if it's the case here, the OP will have to profile. – H H Feb 09 '11 at 09:21

score 3 · Accepted Answer · answered Feb 08 '11 at 12:28

One problem with that approach is that it will turn the entire string into lower case, not just make a case insensetive replace.

You can use a regular expression to do a case insensetive match:

string result = Regex.Replace(
  Filter,
  "</?filter>",
  String.Empty,
  RegexOptions.IgnoreCase
);

Another alternative is to use the IndexOf method to locate the strings, as it can do a case insensetive search:

string result = Filter;
int index;
while ((index = IndexOf("<filter>", StringComparison.OrdinalIgnoreCase)) != -1) {
   result = result.Remove(index, 8);
}
while ((index = IndexOf("</filter>", StringComparison.OrdinalIgnoreCase)) != -1) {
   result = result.Remove(index, 9);
}

score 1 · Answer 3 · answered Feb 08 '11 at 12:24

1

Replace calls to unmanaged code which is implemented in C++ which I imagine will be hard to beat.

However, I can see you keep using .ToLower() which you can cut down to one call and keeping the string.

answered Feb 08 '11 at 12:24

Aliostad

80,612
21
160
208

I think the ToLower() stuff could be a problem. Also, the output string is now lowercase. – Daren Thomas Feb 08 '11 at 12:25
Yes, I was just typing the words :) – Aliostad Feb 08 '11 at 12:25
2

@Henk I used reflector-> it calls to `ReplaceInternal` which is an `extern` interop method. – Aliostad Feb 08 '11 at 12:39

score 1 · Answer 4 · answered Feb 08 '11 at 12:25

1

In any case, you are lower-casing your original string here, which might not be a good thing?

answered Feb 08 '11 at 12:25

bjornars

1,506
2
10
13

The comments in the code indicate that the intention is to perform a case-insensitive replacement. I assume that's why the string is being converted to lower case. – Cody Gray - on strike Feb 08 '11 at 12:29
1

@Cody Gray - That is true, but the problem here is that the part of the string that is not replaced, it also converted to all lower-case. I think that is bjornars concern here. (even if this is more of a comment than an answer actually) – Øyvind Bråthen Feb 08 '11 at 12:35

score 1 · Answer 5 · answered Feb 08 '11 at 12:31

1

It depends on a few things, how long the Filter string is etc.
So you will have to measure.

But I would expect a (single!) RegEx to be faster here.

answered Feb 08 '11 at 12:31

H H

263,252
30
330
514

What if you have 50 strings to replace here for some reason. Won' tthe RegEx be incredible messy? – Øyvind Bråthen Feb 08 '11 at 12:39
@Oyvind: It would be very fast, the more the better. – H H Feb 08 '11 at 12:43
I wasn't referring to speed, but the readability of the code. Any way to make that look even remotely good? (I don't now RegEx, that's why I'm asking ;) ) – Øyvind Bråthen Feb 08 '11 at 12:53
@Oyvind: `@"\|\"` . Could even shorten that to `@"\?filter\>"` but the `|` gives the general pattern: `text1|text2|text3`, and then escape the RegEx tokens – H H Feb 08 '11 at 12:55

astellin · Answer 6 · 2011-02-08T12:57:17.717

0

If the provided code works for you, than this will be faster:

private void MyMethod(string Filter)
{
   //need to remove <Filter> and </Filter> case in-sensitive
   var result = Filter.ToLower().Replace("</filter>","");

  ...........................
}

as the first statement's result is ignored.

edited Feb 08 '11 at 12:57

answered Feb 08 '11 at 12:27

astellin

433
3
13

You're missing a closing double quote. Also, this doesn't produce the same result. It just removes one of the steps. Essentially, your code isn't doing what the comment above it says. – David Feb 08 '11 at 12:36
True, but it IS doing the same thing as the provided code, but faster, as both replaces are done on `Filter.ToLower()`. – astellin Feb 08 '11 at 12:59
I'm having a hard time not upvoting this. I shouldn't and won't. But you made me smile :) – Daren Thomas Feb 08 '11 at 14:18
You are correct in that the original code actually does produce a result that is discarded. However that is most likely not what was intended with the code, as it doesn't make much sense. – Guffa Feb 08 '11 at 14:18

Out of curiosity - is there a better performing approach to do this string replace?

6 Answers6