Search code examples
vb.netwinformsvisual-studio-2017monoraspberry-pi3

Is it necessary to instantiate a new custom eventarg each time the event is fired?


I have a WinForms application that is using events with custom eventargs to update a status panel on the MainForm.

I declare my event in my class like so:

Public Event BomNotLoaded As EventHandler(Of StatusEventArgs)

When the event is fired, I call it like so:

OnBomNotLoaded(New StatusEventArgs(2, "Please scan a SKU/UPC First!", Color.Red, Color.White))

Protected Overridable Sub OnBomNotLoaded(ByVal e As StatusEventArgs)
    RaiseEvent BomNotLoaded(Me, e)
End Sub

In the MainForm code, I have this:

Private Sub _station_BomNotLoaded(sender As Object, e As StatusEventArgs) Handles _station.BomNotLoaded
    SetStatus(e)
    _alert.InvalidScan()
End Sub

The SetStatus method then uses the values in StatusEventArgs to set the text of a label, an image, and the label bakcground color and the label text color. The image is derived from the first number as a code number.

Is it necessary for me to instantiate a new StatusEventArgs every time this event is fired? Is there another way that could offer better performance?

The reason I ask is this application is running on a RaspberryPi3 under Mono and I am having performance issues with high cpu usage.


Solution

  • In truth it shouldn't hurt to have a single object created as initialization that is modified and then passed to every event call. The Event will create and send a copy of a pointer to the EventArgs object when it's raised. The Event Args is a ByVal declaration after all. In standard behavior the EventArgs object is created used, and then disposed of. In this situation, for a few cycles there's actually 2 reference to the EventArgs object in memory. The heavy weight of the various properties are maintained consistently though as opposed for just while needed for use. This also risks the eventargs instance being modified by the calling code before it can be used by the event handler.

    Now, for changing behaviors, there are a few options, and options means choices, benefits and drawbacks.

    The first one is bad advice, but doable. If you check in the Event Handler for the event object being NOTHING before using, then you can raise the event with the eventargs being nothing. But this is a bad design paradigm as it's unexpected. We assume that the EventArgs object is going to be something.

    Private Sub _station_BomNotLoaded(sender As Object, e As StatusEventArgs) Handles _station.BomNotLoaded
       If e IsNot Nothing Then 
          SetStatus(e)
          _alert.InvalidScan()
       End If 
    End Sub
    

    Like I said, I see this as a bad idea as it behaves in a way that is unexpected.

    Alternatively, if you modify/create what events you need, then you get to describe the event signature for each of those events.

    If you have a add an event such as "BomNotScanned" which doesn't have an Event argument class associated with it, then you can raise that event and whatever handles it for the specific scenario of the SKU/UPC not being scanned. Then you could still have your BomNotLoaded for other scenarios where the data is not as expected.

    This has the drawback of making the event structure more complex. In some situations that's expected, in others not.

    Another possibility is to throw Exceptions in those cases where the data is not as expected (such as the SKU/UPC not being scanned) and use Events in cases only where the data is 'good.' In that scenario the using code handles it inline in the function as opposed to an event that pops up elsewhere. This has a drawback in that exceptions include stack information, and can be heavier against performance than events.

    Finally there are a few options in how you're currently doing things which can impact performance in small ways.

    First, is that you're passing contextual clues on the state of the event from the object to the form. Your StatusEventArg has those 4 parameters, and two of them are colors which I'm assuming you're using for the alert message. The first I assuming is some type of identification flag.

    IMO, you should have your UI flag those colors based upon the identification flag (i.e. 2 gives you red background, with white text that is bold, while a 1 would show black text in normal weight on a white background, etc). That creates a separation of concerns, and stops the scanner class from dictating how the main form responds to things.

    Likewise, dropping the description string in place of a defined enumeration flag means that you're passing less information back and forth which increases efficiency. This means that instead of sending 3 integers and a string from the class to the form, you're only sending one integer.

    Finally, you could build your NEW method for the StatusEventArgs so that it doesn't accept parameters and just defines a certain base state (say a Good state). Then you can use WITH (as an object initializer) to set just those parameters you need for any given scenario.

    For example, consider this class definition:

    Public class StatusEventArgs
       Inherits EventArgs 
    
       Public Sub New()
         Flag = 1 
         Random1 = "Random Text" 
         Random2 = ""
       End Sub 
    
       Public Property Flag AS Integer 
       Public Property Random1 As String
       Public Property Random2 As String 
    End Class
    

    I can then instantiate it in these ways:

    Dim se1 As new StatusEventArgs With {.Flag = 2, .Random1="Bob Denver"} 
    Dim se2 As New StatusEventArgs With {.Random1 = "Hello", .Random2="World" }
    

    In truth, this doesn't really save you anything performance wise, but it does mean that you only need the value definitions where you're changing something from the default.

    All of this said, I'm uncertain on any aspect of the defined logic which could generate performance concerns unless you're raising the event too often. That said, I'm not overly familiar with embedded systems such as Raspberry PI.