Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Since I've started to use a computer, I hated everytime an application's UI stops responding for a while. However, there are some applications I see, that are doing their "heavy" jobs, yet the UI remains responsive, when the proccess is completed the result is shown allowing the user do something else meanwhile.

So, as my project is growing bigger and bigger, I started to suffer from the same issue. I think I have a general idea about the underlying logic of crossthreads. However, the sample code I have below doesn't seem to me to be the proper way of doing things. Implementing each function takes so much code. For all the functions accepting diffrent kind and number of parameters, creating a delegate, and making them invokable are very long jobs. For a already written project, this is even worse.

Here is my little template that changes the text of the form from another thread contained in another class. Sorry for the messy code, I hope it is clear enough.

Public Class Form1

    Dim x As ClassA

    Private Sub Form1_HandleCreated(sender As Object, e As System.EventArgs) Handles Me.HandleCreated
        x = New ClassA(Me.Handle)
    End Sub

    Private Sub Button1_Click(sender As System.Object, e As System.EventArgs) Handles Button1.Click
        x.StartAThread()
    End Sub
End Class

Public Class ClassA

    Dim MainForm As Form1

    Sub New(handle As IntPtr)
        MainForm = CType(Form1.FromHandle(handle), Form1)
    End Sub

    Public Sub StartAThread()
        Dim newthread As New Threading.Thread(AddressOf threadsub)
        newthread.IsBackground = True
        newthread.Start()
    End Sub

    Private Delegate Sub dnoparam()
    Dim dabc As New dnoparam(AddressOf abc)

    Private Sub threadsub()
        If MainForm.InvokeRequired Then
            MainForm.Invoke(dabc)
        Else
            abc()
        End If
    End Sub

    Private Sub abc()
        MainForm.Text = "blabla"
    End Sub

End Class

What I am asking is: Is the code above contains the true pattern of doing things? Is this a coding problem for everyone? Is there a better way? What path the professional products are following for keeping the UI as responsive as possible?

share|improve this question
add comment

2 Answers 2

up vote 3 down vote accepted
  1. You are mixing threading, computation and displaying the value into one class, you shouldn't do that. Read about separation of concerns.

  2. In most cases, it's better not to use Thread directly, but instead use something like BackgroundWorker or Task. They offer better capabilities and are more efficient.

    To do this with Tasks, you would need to use StartNew(), ContinueWith() and TaskScheduler.FromCurrentSynchronizationContext().

  3. You don't need to create new delegate type for each combination of parameters, such delegate types are already in the framework: Action for Subs and Func for Functions.

  4. You don't actually need to use separate delegate type for each parameter combination, if you use lambdas.

Putting all this together, the code could look something like (using Tasks, but BackgroundWorker could be used instead):

Task.Factory.StartNew(Function() foo.CalculateResult(parameter)) _
    .ContinueWith(Sub(resultTask) MainForm.Text = resultTask.Result,
                  TaskScheduler.FromCurrentSynchronizationContext())

If this is still too much code for you, you can put it into a reusable method:

Public Sub CalculateAndShow(Of T)(calculate As Func(Of T), show As Action(Of T))
    Task.Factory.StartNew(calculate) _
        .ContinueWith(Sub(resultTask) show(resultTask.Result),
                      TaskScheduler.FromCurrentSynchronizationContext())
End Sub

You would then use it like this:

CalculateAndShow(Function() foo.CalculateResult(parameter), Sub(result) MainForm.Text = result)

Also, if you can use C# 5.0, doing this is even easier (this assumes you make your event handler Async):

MainForm.Text = Await Task.Run(Function() foo.CalculateResult(parameter))
share|improve this answer
    
Excellent answer. –  codesparkle Jan 6 '13 at 14:16
add comment

You can disable the cross thread exception by cheating a little. Since its old code it should be ok. Try not to use it on new code.

Disable cross thread checking:

System.Windows.Forms.Form.CheckForIllegalCrossThreadCalls = false;

Instead of coding threads like you do, use the BackgroundWorker class which is created for threading in Forms.

http://msdn.microsoft.com/en-us/library/cc221403(v=vs.95).aspx

Using BackgroundWorker you do not have to do that little cheat either, but nice to know. It worked like that before on .net 1.0. There were no errors chrossthreading.

share|improve this answer
    
What do you mean by old code? Disabling the exception makes me somewhat uncomfortable. I will take a look at BackgroundWorker. –  Gun Deniz Jan 6 '13 at 11:11
    
"old code" as in what you have already coded. I understand it as you are trying to update your code to be more responsive. BacktroundWorker is the key to that. –  Wolf5 Jan 6 '13 at 11:38
    
I will give it a try ASAP. Thanks. –  Gun Deniz Jan 6 '13 at 11:48
    
Suggesting to use CheckForIllegalCrossThreadCalls = false is just asking for problems. It means you will most likely introduce subtle threading bugs into your code. –  svick Jan 6 '13 at 13:22
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.