Using of Thread within a constructor?

Question

I am a newbie and trying to learn the right way.

Is it acceptable to use a Thread within a constructor to avoid gui(Form) to freeze when an object is created? I will reuse this class often.

class Cmd
{
    protected static string parameters;
    protected HashSet<string> list_result;

    public Cmd( string parameters)
    {
        Thread Thread1 = new Thread(new ThreadStart(Process1));
        Thread1.Start();
        Thread1.Join();
    }

     private void Process1()
    {
        ProcessStartInfo processStartInfo = new ProcessStartInfo("cmd", "/c " + parameters);
        processStartInfo.RedirectStandardOutput = true;
        processStartInfo.RedirectStandardError = true;
        processStartInfo.CreateNoWindow = true;
        processStartInfo.UseShellExecute = false;

        Process process = Process.Start(processStartInfo);

        list_result = new HashSet<string>();
        while (!process.StandardOutput.EndOfStream)
        {
            string line = process.StandardOutput.ReadLine();
            list_result.Add(line);
        }
    }

The way you've written the code here won't help one bit since you join on the thread, effectively blocking until the thread is done. Did this actually solve anything? — Lasse V. Karlsen, Jun 10 '15 at 11:54
I wouldn't join on the thread - not in the constructor. Can't you use a `continuation`, at least? You'd need to use a `task` rather than your `System.Thread`, but for what you're trying to do, that should be fine, IMHO... — code4life, Jun 10 '15 at 11:55
Instead of a separate class, consider using `Task.Run` to run your method. You can call `await` in your UI to await asynchronously for the other process to finish. — Panagiotis Kanavos, Jun 10 '15 at 12:06
Or you could use `ReadLineAsync` instead of `ReadLine` and avoid blocking without using a thread at all — Panagiotis Kanavos, Jun 10 '15 at 12:08
It works fine and I only wanted to know if this practice is acceptable. join() is needed since the object has to be created before any methods are called. As advice, I will try Task since it looks simple to implement. — , Jun 10 '15 at 12:53

score 3 · Answer 1 · answered Jun 10 '15 at 12:19

You don't even need a thread for this. You can use the StreamReader's asynchronous methods to read the input lines asynchronously:

    private async void button1_Click(object sender, EventArgs e)
    {
        var lines=await Process1(@"dir g:\ /s");
        var result= String.Join("|", lines);
        this.textBox1.Text = result;
    }

    private async Task<HashSet<String>>   Process1(string parameters)
    {
        var list_result = new HashSet<string>();
        ProcessStartInfo processStartInfo = new ProcessStartInfo("cmd", "/c " + parameters);
        processStartInfo.RedirectStandardOutput = true;
        processStartInfo.RedirectStandardError = true;
        processStartInfo.CreateNoWindow = true;
        processStartInfo.UseShellExecute = false;

        Process process = Process.Start(processStartInfo);


        while (!process.StandardOutput.EndOfStream)
        {
            string line = await process.StandardOutput.ReadLineAsync();
            list_result.Add(line);
        }
        return list_result;
    }

The advantage is that you don't waste a thread, you don't need any synchronization code or static fields to pass the parameters and read the results.

Denis Yarkovoy · Answer 2 · 2015-06-10T12:29:55.817

0

If you would like to avoid freeze in your UI when doing a time-consuming task, you should add BackgroundWorker to your form and run your task in its event handler

You can find the example here: https://msdn.microsoft.com/en-us/library/cc221403%28v=vs.95%29.aspx

You should also consider using newer async/await logic which is usually better than BackgroundWorker, as Panagiotis Kanavos mentioned in his answer

edited Jun 10 '15 at 12:29

answered Jun 10 '15 at 11:59

Denis Yarkovoy

1,277
8
16

1

`Task.Run` and `async/await` are easier and allow composing multiple asynchronous calls, exception handling etc. BackgroundWorker is useful only for compatibility reasons – Panagiotis Kanavos Jun 10 '15 at 12:04
@Panagiotis Kanavos - agreed, these can be used as well. – Denis Yarkovoy Jun 10 '15 at 12:07
@Panagiotis Kanavos Why do you say those options are easier than `BackgroundWorker`? I've found it easy to use while at least `async/await` seems fairly confusing. Maybe I just haven't read the right documentation. – bkribbs Jun 10 '15 at 12:12
@bkribbs check my answer to see how this can be done using `async/await` without any threading at all. In fact, BackgroundWorker makes it almost impossible to perform two separate asynchronous actions (eg download a file and write it to a database, or make two web service calls) - you'd need two separate workers, or perform everything synchronously. – Panagiotis Kanavos Jun 10 '15 at 12:22
@bkribbs: I have a blog post series on [using `Task.Run` instead of `BackgroundWorker`](http://blog.stephencleary.com/2013/05/taskrun-vs-backgroundworker-intro.html). – Stephen Cleary Jun 10 '15 at 12:27
@Stephen Cleary Just read the article. It makes sense, but as a fairly new C# programmer, seeing all the await keywords, new types like IProgress, and lambda expressions that so frequently seem to be used with `Task.Run`, I can easily see why I chose `BackgroundWorker` back when I last needed multithreading. It's just intimidating and `BackgroundWorker` did what I need. That said, I see why `Task.Run` is advantageous and will try it out on my next multi-threading code to see how it compares. Thanks for the link! – bkribbs Jun 10 '15 at 12:56

Using of Thread within a constructor?

2 Answers2