5

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>,"");

  ...........................
}
Jon
  • 35,719
  • 73
  • 218
  • 368
  • 1
    `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
  • 3
    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
  • 3
    `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 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 Feb 08 '11 at 12:34
  • 4
    @Ø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 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

6 Answers6

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.

Community
  • 1
  • 1
Daren Thomas
  • 61,662
  • 38
  • 143
  • 192
3

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);
}
Guffa
  • 640,220
  • 96
  • 678
  • 956
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.

Aliostad
  • 76,981
  • 19
  • 152
  • 203
1

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

bjornars
  • 1,466
  • 1
  • 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 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
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.

Henk Holterman
  • 236,989
  • 28
  • 287
  • 464
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.

Excel20
  • 411
  • 2
  • 12
  • 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()`. – Excel20 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