views:

137

answers:

5

Here I am storing the elements of a datagrid in a string builder using a for loop, but it takes too much time when there is a large number of rows. Is there another way to copy the data in to a string builder in less time?

for (int a = 0; a < grdMass.RowCount; a++)
     {
         if (a == 0)
             {
                 _MSISDN.AppendLine("'" + grdMass.Rows[a].Cells[0].Value.ToString() + "'");

              }
          else
             {
               _MSISDN.AppendLine(",'" + grdMass.Rows[a].Cells[0].Value.ToString() + "'");
             }
     }
+3  A: 

There is no way to improve this code given the information you have provided. This is a simply for loop that appends strings to a StringBuilder - there isn't a whole lot going on here that can be optimized.

This may be one of those cases where something takes a long time simply because you are processing a lot of data. Perhaps there is a way to cache this data so you don't have to generate it as often. Is there anything else you can tell us that would help us find a better way to do this?


Side note: It is very important that you validate your suspicions as to the particular section of code that is causing the slowness. Do this by profiling your code so that you don't spend time trying to fix a problem that exists elsewhere.

Andrew Hare
A: 

StringBuilder is pretty much as fast as it gets for building up strings -- and that is pretty goshdarned fast. If StringBuilder is too slow, you are probably trying to process too much data in one go. Are you sure it is really the string building which is slow and not some other part of the processing?

One tip that will speed up StringBuilder for very large strings: set the capacity up front. That is, call the StringBuilder(int) constructor instead of the default constructor, passing an estimate of the number of characters you plan to write. It will still expand if you underestimate -- this just saves the initial "well, 1K wasn't enough, time to allocate another 2K... 4K... etc." But this will make only a small difference, and only if your strings are very long.

itowlson
+3  A: 

As others have said, the StringBuilder is about as fast as you're going to get, so assuming this is the only bit of code that could be causing your slow down, there's probably not much you can do... but you could slightly optimise it by removing the small amount of string concatenation you are doing. I.e:

for (int a = 0; a < grdMass.RowCount; a++)
{
    if (a == 0)
    {
        _MSISDN.Append("'");
    }
    else
    {
        _MSISDN.Append(",'");
    }
    _MSISDN.Append(grdMass.Rows[a].Cells[0].Value);
    _MSISDN.AppendLine("'");
}

Edit: You could also clean up the if statement (although I highly doubt it's having a noticable effect) like so:

//First row
if (grdMass.RowCount > 0)
{
    _MSISDN.Append("'");
    _MSISDN.Append(grdMass.Rows[0].Cells[0].Value);
    _MSISDN.AppendLine("'");
}
//Second row onwards
for (int a = 1; a < grdMass.RowCount; a++)
{
    _MSISDN.Append(",'");
    _MSISDN.Append(grdMass.Rows[a].Cells[0].Value);
    _MSISDN.AppendLine("'");
}
Alconja
A: 

This would be better....

 if (grdMass.RowCount > 0)
 {
      _MSISDN.AppendLine("'" + grdMass.Rows[0].Cells[0].Value.ToString() + "'");

      for (int a = 1; a < grdMass.RowCount; a++)
      {
            _MSISDN.AppendLine(",'" + grdMass.Rows[a].Cells[0].Value.ToString() + "'");
      }     
 }
Nescio
A: 

I'm suspecting that it's not the string building that takes a long time, perhaps it's accessing the grid elements that is slow.

You could rewrite your code like this:

var cellValues = grdMass.Rows
    .Select(r => "'" + r.Cells[0].Value.ToString() + "'")
    .ToArray();

return String.Join(",", cellValues);

Now you can verify which part takes the most time. Is it building the cellValues array, or is it the String.Join call?

Geert Baeyaert
You could be right, but stackshots would prove it.
Mike Dunlavey