Optimization of method for string replacement

Question

Can anyone help me how to optimize 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;
}

Thanks in advance!

Answer 1

In this code you are scanning the 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 for a char instead of a string:

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

---

Here you are doing several replaces with the same replacement:

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

Instead you can use a regular expression to match all of them:

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

(Whether this actually gives better performance has to be tested with some of your actual data. Eventhough it's less operations, there is some overhead in setting up the regular expresson.)

---

You are using a loop to reduce clusters of characters:

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

Instead you can use a regular expression to make a single replacement:

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

Answer 2

Your most obvious first step would be to use a StringBuilder instead of a String.

String is an immutable type. That means that once created, its value never changes. So, for every Replace call you make in your method, the program will create a whole new String instance to store the result, which is both memory- and processor-intensive. (I say this comparitively - you're not going to max your machine out calling this method once, but if you're calling it thousands of times you'll certainly notice!)

StringBuilder, on the other hand, is a class designed to manipulate strings in-memory and doesn't have to copy/recreate the memory every time you change the String.

So, one big step in the right direction would be to use this at the start 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);

Notice that I also did the .ToLower() earlier because StringBuilder doesn't have an equivalent, and also notice the sb.Append, which again will prevent a lot of rewriting.

This isn't as optimal as it could get, but it should be quite an improvement...

The one thing I missed out was replacing the "--". StringBuilder doesn't have a "Contains" function, but you could use a regular expression to catch everything in one pass (instead of needing the loop).

Hope that gets you started!