CODEWITHSUNDEEP
c#performanceloops
Kenny Thompson
Kenny Thompson

Reputation: 1492

Why is this loop so slow?

I have tried to look up a reason as to why this loop is so slow, but I havent gotten a good answer yet. The following loop takes over a minute to execute:

        string answer = "";
        string headers = "";
        string datarows = "";
        bool firstRun = true;
        foreach (Dictionary<string, string> row in JSON)
        {
            datarows += "<tr>";
            foreach (KeyValuePair<String, String> cell in row)
            {
                if (firstRun) { headers += "<th>" + cell.Key + "</th>"; }
                datarows += "<td>" + cell.Value + "</td>";
            }
            datarows += "</tr>";
            firstRun = false;
        }
        answer += "<table><tr>" + headers + "</tr>" + datarows + "</table>";
        return answer;

The JSON variable is a List and contains about 1150 dictionaries. Each dictionary contains 9 key value pairs. Any thoughts?

Upvotes: 1

Views: 348

Answers (4)

Shyju
Shyju

Reputation: 218932

Did you try changing the string type to StringBuilder where you have to concatenate ?

http://www.dotnetperls.com/stringbuilder-performance

StringBuilder answer = new StringBuilder();
StringBuilder headers = new StringBuilder();
StringBuilder datarows = new StringBuilder();
bool firstRun = true;
foreach (Dictionary<string, string> row in JSON)
{
    datarows .Apeend("<tr>");
    foreach (KeyValuePair<String, String> cell in row)
    {
        if (firstRun) { headers.Apeend("<th>" + cell.Key + "</th>"); }
        datarows.Append("<td>" + cell.Value + "</td>");
    }
     datarows.Append("</tr>");
    firstRun = false;
}
answer.Append("<table><tr>" + headers + "</tr>" + datarows + "</table>");
return answer.toString();

Upvotes: 1

Haedrian
Haedrian

Reputation: 4328

Whenever you append to a string, the old one is destroyed, a new string is created.

So the string concatination gets slower and slower the more you put it.

If you swap for a StringBuilder you should get much better speed.

Upvotes: 3

Garry Marsland
Garry Marsland

Reputation: 1221

Trying using a StringBuilder instead of manually concatenating the strings.

The way you are doing it, the string is re-evaluated every time you add more to the end of it and rebuilt every time, which is costly. The StringBuilder is much much more efficient for this type of work.

Upvotes: 4

Rob Levine
Rob Levine

Reputation: 41358

The obvious issue that springs out is your string concatenation.

Every time you append to a string, you are actually appending to a copy of the string (as the strings individually are immutable). This can be extremely costly.

You should prefer either a StringBuilder, or, for generating HTML like this, you might want to investigate the HtmlTextWriter - this will help take care of the "well-formed-ness" of the HTML amongst other things.

Upvotes: 12

Related Questions