Optimizing a method for replacing strings

Question

Can someone help me in optimizing this method?

public override VirtualPathData GetVirtualPath(RequestContext requestContext, RouteValueDictionary values) { VirtualPathData path = base.GetVirtualPath(requestContext, values); if (path != null) { string virtualPath = path.VirtualPath; string condition = string.Empty; if (virtualPath.Contains("?")) { condition = virtualPath.Substring(virtualPath.IndexOf("?")); virtualPath = virtualPath.Substring(0, virtualPath.IndexOf("?")); } virtualPath = virtualPath.Replace(@"%C5%BD", "ž"); virtualPath = virtualPath.Replace(@"%C4%90", "đ"); virtualPath = virtualPath.Replace(@"%C4%86", "ć"); virtualPath = virtualPath.Replace(@"%C4%8C", "č"); virtualPath = virtualPath.Replace(@"%C5%A0", "š"); virtualPath = virtualPath.ToLower().Replace(",", "-").Replace("%20", "-").Replace("&", "-"); virtualPath = virtualPath.Replace(@"-amp;", "&"); while (virtualPath.Contains("--")) { virtualPath = virtualPath.Replace("--", "-"); } path.VirtualPath = virtualPath + condition; } return path; } 

Answer 1

In this code, you scan a string three times for a character:

 if (virtualPath.Contains("?")) { condition = virtualPath.Substring(virtualPath.IndexOf("?")); virtualPath = virtualPath.Substring(0, virtualPath.IndexOf("?")); } 

Instead, scan it once and use the result three times. Also scan char instead of string:

 int pos = virtualPath.IndexOf('?'); if (pos != -1) { condition = virtualPath.Substring(pos); virtualPath = virtualPath.Substring(0, pos); } 

---

Here you make several replacements with the same replacement:

 virtualPath = virtualPath.ToLower().Replace(",", "-").Replace("%20", "-").Replace("&", "-"); 

Instead, you can use regex for all of them:

 virtualPath = Regex.Replace(virtualPath.ToLower(), "(,|%20|&)", "-"); 

(Regardless of whether this actually gives the best performance, you need to check some of your actual data. Although this is less than the operations, there is some overhead when setting up the regular expression.)

---

You use a loop to reduce character clusters:

 while (virtualPath.Contains("--")) { virtualPath = virtualPath.Replace("--", "-"); } 

Instead, you can use regex to create one replacement:

 virtualPath = Regex.Replace(virtualPath, "-{2,}", "-"); 

Answer 2

The most obvious first step is to use StringBuilder instead of String.

A string is an immutable type. This means that once created, its meaning never changes. Thus, for each Replace call that you make in your method, the program will create a whole new instance of String to store the result, which will be both intense and memory intensive. (I say this comparatively - you are not going to maximize your machine by calling this method once, but if you call thousands of times, you will definitely notice!)

StringBuilder, on the other hand, is a class designed to manipulate strings in memory and should not copy / recreate memory every time you change a string.

So, one big step in the right direction would be to use this at the beginning of your method:

  StringBuilder sb = new StringBuilder(path.VirtualPath.ToLower()); string condition = string.Empty; int index = path.VirtualPath.IndexOf("?"); if (index > -1) { condition = virtualPath.Substring(pos); sb.Remove(0, index); } sb.Replace(@"%C5%BD", "ž") .Replace(@"%C4%90", "đ") .Replace(@"%C4%86", "ć") .Replace(@"%C4%8C", "č") .Replace(@"%C5%A0", "š") .Replace(",", "-") .Replace("%20", "-") .Replace("&", "-") .Replace(@"-amp;", "&"); sb.Append(condition); 

Note that I also did .ToLower () earlier, because StringBuilder has no equivalent, and also pay attention to sb.Append, which again will prevent a lot of rewriting.

This is not as optimal as it could be, but it should be a pretty good improvement ...

The only thing I missed is the substitution "-". StringBuilder does not have a Contain function, but you can use a regular expression to catch everything in one pass (instead of having to loop).

Hope you started!