Reputation: 602
I'm running this code and it freezes my program for a while to fill the Textbox. Is there a way to do it faster? Or if it is not possible to make it faster by running in background so I can continue using the program?
var numbers = "0123456789";
var q = numbers.Select(x => x.ToString());
int size = 4;
for (int i = 0; i < size - 1; i++)
q = q.SelectMany(x => numbers, (x, y) => x + y);
foreach (var item in q)
{
//Console.WriteLine(item);
textBox.Text += item + Environment.NewLine;
}
Upvotes: 1
Views: 88
Reputation: 35891
The main problem seems concatenating the strings directly to the control, which refreshes itself on each modification of its Text
property. Try this version:
var result = string.Empty;
foreach (var item in q)
{
result += item + Environment.NewLine;
}
textBox.Text = result;
You could also use StringBuilder
but on my machine difference was meaningless (EDIT : apparently not, details below). And of course you should in general use BackgroundWorker
for heavy computations. This is the boundary case IMHO for modern machines.
As a follow-up on discussion with Pierre-Luc Pineault under his answer I did some measurements, and this is what came out:
StringBuilder average: 0,9336, stddev: 0,591938375170962 [ms]
String average: 61,9763, stddev: 7,17896498877082 [ms]
So yes, indeed you should use StringBuilder
even in this case:
var result = new StringBuilder();
foreach (var item in q)
{
result.Append(item + Environment.NewLine);
}
textBox.Text = result.ToString();
Here is the code I've used to make the measurements:
class Test
{
private static Stopwatch sw = new Stopwatch();
public static int st()
{
var numbers = "0123456789";
var q = numbers.Select(x => x.ToString());
int size = 4;
Stopwatch sw = new Stopwatch();
sw.Start();
for (int i = 0; i < size - 1; i++)
q = q.SelectMany(x => numbers, (x, y) => x + y);
String s = "";
foreach (var item in q)
s += item;
return s.Length;
}
public static int sb()
{
var numbers = "0123456789";
var q = numbers.Select(x => x.ToString());
int size = 4;
Stopwatch sw = new Stopwatch();
sw.Start();
for (int i = 0; i < size - 1; i++)
q = q.SelectMany(x => numbers, (x, y) => x + y);
StringBuilder sb = new StringBuilder();
foreach (var item in q)
sb.Append(item);
return sb.ToString().Length;
}
static void Main()
{
int n = 100000;
List<long> sbSamples = new List<long>(n);
List<long> stSamples = new List<long>(n);
for (int i = 0; i < n; ++i)
{
sw.Reset();
sw.Start();
sb();
sw.Stop();
sbSamples.Add(sw.ElapsedMilliseconds);
sw.Reset();
sw.Start();
st();
sw.Stop();
stSamples.Add(sw.ElapsedMilliseconds);
}
double sbAv = 1.0 * sbSamples.Sum() / sbSamples.Count;
double stAv = 1.0 * stSamples.Sum() / sbSamples.Count;
double sbStdDev = Math.Sqrt(sbSamples.Select(x
=> (x - sbAv) * (x - sbAv)).Sum() / sbSamples.Count);
double stStdDev = Math.Sqrt(stSamples.Select(x
=> (x - stAv) * (x - stAv)).Sum() / stSamples.Count);
Console.WriteLine(
"StringBuilder average: " + sbAv + ", stddev: " + sbStdDev);
Console.WriteLine(
"String average: " + stAv + ", stddev: " + stStdDev);
Console.ReadKey();
}
}
Upvotes: 1
Reputation: 9191
Using a background worker will not solve the underlying problem; you're not filling the textbox correctly.
By using the +=
string operator directly in a loop, you're using Schlemiel the Painter's algorithm, which is highly inefficient.
You can solve this by using StringBuilder and only show the end result in the textbox:
StringBuilder sb = new StringBuilder();
foreach (var item in q)
{
sb.Append(item + Environment.NewLine);
}
textBox.Text += sb.ToString();
Iterating over your 10k numbers shouldn't take more than a few milliseconds, at most.
Upvotes: 4