Pattern to handle threads status in C#

I am currently developing a program that must handle multiple threads. When I start the program I run multiple threads (my example is limited to one). I have to display their status in a single TextBox. I opted for the next solution. Is this way is correct? Are there any other pattern? The Observer perhaps? I can't find a good way to do this on the web.

namespace ThreadTest
{
    public partial class Form1 : Form
    {
        // This delegate enables asynchronous calls for setting
        // the text property on a TextBox control.
        delegate void ChangedCallback(object sender, JobEventArgs e);

        public Form1()
        {
            InitializeComponent();
        }

        private void Form1_Load(object sender, EventArgs e)
        {
            MyThread myThread = new MyThread();
            myThread.Changed += new MyThread.JobEventHandler(myThread_Changed);

            // Create the thread object, passing in the Alpha.Beta method
            // via a ThreadStart delegate. This does not start the thread.
            Thread oThread = new Thread(new ThreadStart(myThread.MyJob));

            // Start the thread
            oThread.Start();
        }

        void myThread_Changed(object sender, JobEventArgs e)
        {
            // InvokeRequired required compares the thread ID of the
            // calling thread to the thread ID of the creating thread.
            // If these threads are different, it returns true.
            if (this.textBox1.InvokeRequired)
            {
                ChangedCallback d = new ChangedCallback(myThread_Changed);
                this.Invoke(d, new object[] { sender, e });
            }
            else
            {
                // Display the status of my thread
                textBox1.Text += e.Counter;
            }
        }
    }

    public class MyThread
    {
        // A delegate type for hooking up change notifications.
        public delegate void JobEventHandler(object sender, JobEventArgs e);

        // An event that clients can use to be notified whenever the
        // elements of the list change.
        public event JobEventHandler Changed;

        // Invoke the Changed event; called whenever list changes
        protected virtual void OnChanged(JobEventArgs e)
        {
       开发者_如何转开发     if (Changed != null)
                Changed(this, e);
        }

        public void MyJob()
        {
            for (int i = 0; i < 1000; i++)
            {
                Thread.Sleep(1000);
                JobEventArgs e = new JobEventArgs(i);
                OnChanged(e);
            }
        }
    }

    public class JobEventArgs
    {

        public int Counter { get; set; }

        public JobEventArgs(int i)
        {
            Counter = i;
        }
    }
} 

---

It looks just fine to me. In fact you are using the observer pattern. It's just c#'s nice event syntax eliminating the interface and reducing boilerplate.

However, there is a lot of redundant code and other readability problems in there.

• The this qualifier is redundant when specifying a memeber such as this.textBox1 or this.Invoke(...).
• Try to always specify visibility such as private or public for methods.
• A suitable delegate is created automatically around a method-group enabling a shorthand syntax. For example: new Thread(myThread.MyJob) or myThread.Changed += myThread_Changed.
• Consider using simple Action as event handler delegate types instead of a custom (ChangedCallback). The myThread_Changed method can then just accept an int as single parameter allowing you to drop lots of redundant types.
• To avoid problems make a thread-local copy of the event before checking for null and invoking.

Like this:

JobEventHandler tmp = Changed;
if (tmp != null) tmp(this, e);

---

You code is not fine.

1. Why do you not let your thread class create the thread? It's more logical and gives a nice encapsulation:
2. You should not declare delegates inside a class, it makes refactoring harder.
3. If you are sleeping in the thread, why don't you use a Timer instead?

#

public partial class Form1
{
    delegate void ChangedCallback(object sender, JobEventArgs e);

    public Form1()
    {
        InitializeComponent();
    }

    private void Form1_Load(object sender, EventArgs e)
    {
        MyThread myThread = new MyThread();
        myThread.Changed += myThread_Changed;
        myThread.Start();
    }

    void myThread_Changed(object sender, JobEventArgs e)
    {
        if (this.textBox1.InvokeRequired)
        {
            ChangedCallback d = new ChangedCallback(myThread_Changed);
            this.Invoke(d, new object[] { sender, e });
        }
        else
        {
            textBox1.Text += e.Counter;
        }
    }
}


public class MyThread
{
    private Thread _thread;

    public MyThread()
    {
        _thread = new Thread(WorkerFunc);
    }

    public void Start()
    {
        _thread.Start();
    }

    // use the = {} pattern since you are using multithreading.
    public event JobEventHandler Changed = {};

    protected virtual void OnChanged(JobEventArgs e)
    {
        Changed(this, e);
    }

    private void WorkerFunc()
    {
        for (int i = 0; i < 1000; i++)
        {
            Thread.Sleep(1000);
            JobEventArgs e = new JobEventArgs(i);
            OnChanged(e);
        }
}

// A delegate type for hooking up change notifications.
public delegate void JobEventHandler(object sender, JobEventArgs e);