2

Hoping to get some best-practice advise with regards to capturing a returned message from an instantiated class on my form.

In my form (form1.vb), I have a label which reflects what is being done, with the code below.

Code in form1.vb to display message:

Public Sub DisplayMessage(ByVal Msg as String, ByVal Show as Boolean)
    Application.DoEvents()
    If Show Then
        lblShow.Text = Msg
        lblShow.Refresh()
    End If
End Sub

I have came across three methods so far:

  1. Direct Form Call. In this scenario the class directly calls the form's message routine:

    form1.DisplayMessage("Show This Message", True)
    


  2. RaiseEvent within class. In this scenario form1 is Friends WithEvents of the class sending the message, and the class raises the event to the form.

    **Declared in Form1.vb**
    Friend WithEvents Class1 as New Class1      
    
    **Declared in Class1.vb**
    Public Event SetMessage(ByVal Msg As String, ByVal Show As Boolean)
    
    **Used in Class1.vb**
    RaiseEvent SetMessage("Show This Message", True)
    


  3. Have an EventArgs class handle the event. In this scenario we have an EventArg.vb class which is instantiated whenever we raise the event.

    **Declared in Form1.vb**
    Friend WithEvents Class1 as New Class1    
    
    Private Sub class1_DisplayMessage(ByVal Msg As String, ByVal showAs Boolean, ByRef e As ProgressMessageEventArgs) Handles Class1.SetMessage
        DisplayMessage(Msg, Show)
    End Sub
    


    **Declared in Class1.vb**
    Public Event SetMessage(ByVal msg As String, ByVal Show As Boolean, ByRef e As ProgressMessageEventArgs)

    Protected Sub CaptureMessage(ByVal msg As String, ByVal Show As Boolean)
        RaiseEvent SetMessage(message, ShowList, New ProgressMessageEventArgs(message))
    End Sub

    **Used in Class1.vb**
    RaiseEvent CaptureMessage("Show This Message", True)


    **EventArg.vb created to handle ProgressMessageEventArgs class**
    Public NotInheritable Class ProgressMessageEventArgs
        Inherits System.EventArgs
        Public txt As String

        Public Sub New(ByVal txt As String)
            MyBase.New()
            Me.Text = txt 
        End Sub
    End Class


Scenario 1 is seemingly the simplest, though I was advised against this and asked to raise an event instead. Over time I came across scenario 3 which involves an additional class vs scenario 2.

Therefore, the question is... Between these three methods, which would be the "proper" way of returning a message from a class to the form? Is the additional EventArg class as per scenario 3 necessary since scenario 2 works fine as well?

Many thanks in advance.

John Doe
  • 23
  • 5
  • "Best Practice" depends on the situation. A form of any of the three can work depending on the use case. However, `Application.DoEvents`, default form instances and non standard event signatures are not a part of any best practice – Ňɏssa Pøngjǣrdenlarp Apr 18 '17 at 12:38
  • Worst practice: [Application.DoEvents()](http://stackoverflow.com/a/5183623/832052) – djv Apr 18 '17 at 16:10
  • You should really just be able to do `lblShow.Text = Msg` unless you are running from a different thread. Why the `Application.DoEvents` and `Refresh` in the first place? – djv Apr 18 '17 at 16:25
  • Raising and handling an event is not much different from calling a method. If running in a separate class to which the form holds a reference, then you would want to raise the event from that class because the class has no way of calling a method on the form. If you are multithreading, i.e. the other class executes long running methods on a non UI thread, you will need to handle the invocation of the label update in the form's event handler. So the answer is not simple without more info. I'd be happy to help more if I can. – djv Apr 18 '17 at 16:30
  • @djv - Thanks. Will study that link and remove what is unnecessary on my code. Fairly new to programming and trying to learn beyond just putting a bunch of lines together as long as it runs, hope you could bear with me. When you say "separate class to which the form holds a reference", am I correct to interpret it as that being when we reference a DLL on our project where it returns a message to the calling form? If we have a class module which the form instantiates within the same project - Both raising an event or directly calling the message method from the form will be fine? – John Doe Apr 19 '17 at 02:59
  • Here is a great "big picture" description of what I'm talking about https://softwareengineering.stackexchange.com/a/127290/106469. When dealing with the UI you may also want your business logic to run on another thread - as not to freeze the UI. This is accomplished by running asynchronous code for long running operations. *Usually* an `Application.DoEvents` suggests the UI is overloaded and someone found that it worked to make the UI display. But I can't be sure. – djv Apr 19 '17 at 14:28
  • 1
    @djv - Cheers. I get the idea and will implement it in my coding. Currently the tools I develop are very simple stuff (usually one button, sequential script execution) but I am not happy with the way I code. I would vote your reply as an answer but I don't seem to have voting rights yet.... – John Doe Apr 19 '17 at 15:18
  • See BackgroundWorker as well https://msdn.microsoft.com/en-us/library/system.componentmodel.backgroundworker.aspx – djv Apr 19 '17 at 15:23
  • @JohnDoe made an answer for you. Good luck! – djv Apr 19 '17 at 15:37
  • `RaiseEvent within class` is the correct solution. I could have stopped there, but I sensed the issue with threading so there's my answer – djv Apr 19 '17 at 15:39

1 Answers1

1

My answer is none of the above. Consider this example

Public Class Form1

    Private WithEvents myClass1 As New Class1()

    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
        myClass1.CountTo1000()
    End Sub

    Private Sub MyClass1_Updated(number As Integer) Handles myClass1.Updated
        Me.Label1.Text = number.ToString()
    End Sub

End Class

Public Class Class1

    Public Event Updated(number As Integer)

    Public Sub CountTo1000()
        For i = 1 To 1000
            System.Threading.Thread.Sleep(1)
            RaiseEvent Updated(i)
        Next
    End Sub

End Class

You have a form and a class, and the form has a reference to the class (the class doesn't even know the form exists). Your business logic is performed in the class, and the form is used to input and display information. CountTo1000() is being called directly from the form, which is bad because basically the UI thread is being put to sleep 1000 times, while the class is trying to update the UI by raising the event after each sleep. But the UI never has time to allow the events to happen, i.e. to be updated. Placing an Application.DoEvents() after Me.Label1.Text = number.ToString() will allow the UI to update. But this is a symptom of bad design. Don't do that.

Here is another example with multi-threading

Public Class Form1

    Private WithEvents myClass1 As New Class1()

    ' this handler runs on UI thread
    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
        ' make a new thread which executes CountTo1000
        Dim t As New System.Threading.Thread(AddressOf myClass1.CountTo1000)
        ' thread goes off to do its own thing while the UI thread continues
        t.Start()
    End Sub

    ' handle the event
    Private Sub MyClass1_Updated(number As Integer) Handles myClass1.Updated
        updateLabel(number.ToString())
    End Sub

    ' invoke on UI thread if required
    Private Sub updateLabel(message As String)
        If Me.Label1.InvokeRequired Then
            Me.Label1.Invoke(New Action(Of String)(AddressOf updateLabel), message)
        Else
            Me.Label1.Text = message
        End If
    End Sub

End Class

Public Class Class1

    Public Event Updated(number As Integer)

    Public Sub CountTo1000()
        For i = 1 To 1000
            System.Threading.Thread.Sleep(1)
            RaiseEvent Updated(i)
        Next
    End Sub

End Class

This simple example shows how a thread can be created and run some code off the UI. When doing this, any method call from the non-UI thread must be invoked on the UI if it must access a UI control (Label1). The program runs smoothly since the Thread.Sleep is done on a different thread than the UI thread, with no need for Application.DoEvents, because the UI thread is otherwise doing nothing, and can handle the events being raised by the other thread.

I focused more on threading, but in both examples the design has a form with a class, and the form knows about the class, but the class doesn't know about the form. More about that can be seen here.

See also:

djv
  • 15,168
  • 7
  • 48
  • 72
  • _A better option than Thread nowadays: BackgroundWorker_ - is obsolete option after `async-await` was introduced – Fabio May 02 '17 at 16:32