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 asthis.textBox1
orthis.Invoke(...)
. - Try to always specify visibility such as
private
orpublic
for methods. - A suitable delegate is created automatically around a method-group enabling a shorthand syntax. For example:
new Thread(myThread.MyJob)
ormyThread.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.
- Why do you not let your thread class create the thread? It's more logical and gives a nice encapsulation:
- You should not declare delegates inside a class, it makes refactoring harder.
- 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);
精彩评论