Aman
Aman

Reputation: 602

C# Code execution takes very long

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

Answers (2)

BartoszKP
BartoszKP

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

Pierre-Luc Pineault
Pierre-Luc Pineault

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

Related Questions