AdvancedHMI Software
General Category => Open Discussion => Topic started by: seth350 on January 30, 2018, 04:11:54 PM
-
Good afternoon all,
I have a question regarding event arguments and their correct use.
I have an app that I have created several different EventArgs for different events. Each passing the needed information to the event sub.
The class this sub is located in is declared once from the MainForm. In this sub, I am using StatusArgs as my event argument.
My question is, is it necessary for me to be calling "New StatusArgs" every time I raise an event? Would calling "New StatusArgs" be creating a new instance that eventually uses up resources?
I suppose the real question is, after calling "New StatusArgs" and setting the information in it, does the instance hang out forever in memory?
Would the better option be to create one instance of StatusArgs for the class, and then just set the arguments every time I raise the event? Would doing this essentially overwrite the variables that are stored in the instance?
Public Sub VerifyPart(ByVal scan As String)
Dim partFound As Boolean = False
For Each item As ListViewItem In _list.Items
If scan = item.SubItems.Item(1).Text Then
If CheckOffItem(item) = True Then
RaiseEvent OnPartProcessed(Me, New StatusArgs(0, String.Format("Part# {0} is valid! Please scan the next part.", scan), Color.Green, Color.White))
_audio.ValidScan()
Else
RaiseEvent OnPartProcessed(Me, New StatusArgs(2, String.Format("Part# {0} previously scanned!" & vbNewLine & "Verify your quanity and try again.", scan), Color.Red, Color.White))
_audio.InvalidScan()
End If
partFound = True
End If
Next item
If partFound = False Then
If scan = "NoRead" Then
RaiseEvent OnPartProcessed(Me, New StatusArgs(2, "Scanner did not find a bar code. Try again.", Color.Red, Color.White))
RaiseEvent InvalidLog(Me, New StatusArgs(scan, "Scanner did not find a bar code. No Read."))
Else
RaiseEvent OnPartProcessed(Me, New StatusArgs(2, String.Format("Part# {0} is invalid. Please verify your parts.", scan), Color.Red, Color.White))
RaiseEvent InvalidLog(Me, New StatusArgs(scan, String.Format("Part# {0} was not found in the current parts list.", scan)))
_audio.InvalidScan()
End If
End If
End Sub
''' <summary>
''' Station Status Arguments Class For Passing Information To The Status Panel.
''' </summary>
Public Class StatusArgs
Inherits EventArgs
Private _code As Integer
Private _msg As String
Private _scan As String
Private _backclr As Color
Private _foreclr As Color
''' <summary>
''' Creates a new instance to pass status arguments to the status panel.
''' </summary>
''' <param name="code">Determines what the graphic and color is set to. 0=No Change To Image 1=Scanning Complete 2=Scan Invalid 3=General Alarm 4=Ready</param>
''' <param name="msg">Text to set the status to.</param>
''' <param name="backclr">Specify what color to set backcolor on status panel.</param>
''' <param name="foreclr">Specify what color to set the text to.</param>
Public Sub New(ByVal code As Integer, ByVal msg As String, ByVal backclr As Color, ByVal foreclr As Color)
_code = code
_msg = msg
_backclr = backclr
_foreclr = foreclr
End Sub
''' <summary>
''' Creates new instance of StatusArgs to pass scanned code and message.
''' </summary>
''' <param name="scan">The scanned bar code to pass.</param>
''' <param name="msg">The message to attach.</param>
Public Sub New(ByVal scan As String, ByVal msg As String)
_scan = scan
_msg = msg
End Sub
''' <summary>
''' Indicates what status image/color should be displayed.
''' </summary>
''' <returns>Integer</returns>
Public ReadOnly Property Code As Integer
Get
Return _code
End Get
End Property
''' <summary>
''' Gets the status message.
''' </summary>
''' <returns>String</returns>
Public ReadOnly Property Message As String
Get
Return _msg
End Get
End Property
''' <summary>
''' Gets the scanned code associated with this event.
''' </summary>
''' <returns>String</returns>
Public ReadOnly Property Scan As String
Get
Return _scan
End Get
End Property
''' <summary>
''' Gets the image that the status image should be set to.
''' </summary>
''' <returns>Image</returns>
Public ReadOnly Property Image As Image
Get
Select Case _code
Case 0
Return Nothing
Case 1 'scanning complete
Return My.Resources.greenCheck
Case 2 'invalid scan
Return My.Resources.traffic_light_red
Case 3
Return My.Resources.important
Case 4 'station ready
Return My.Resources.traffic_light_green
Case Else
Return My.Resources.gear
End Select
End Get
End Property
''' <summary>
''' Gets the back color of the status image and message.
''' </summary>
''' <returns>Color</returns>
Public ReadOnly Property BackColor As Color
Get
Return _backclr
End Get
End Property
''' <summary>
''' Gets the fore ground color for the status message.
''' </summary>
''' <returns>Color</returns>
Public ReadOnly Property ForeColor As Color
Get
Return _foreclr
End Get
End Property
End Class
-
The best practice is to create a new instance of your event args for each event. The reason is to be safer in multi-threading. Think of the scenario that an event fires and part way through the event handler, another thread fires the event again. If you used the same event args instance, halfway through the event handler for first firing of the event, your arguments would suddenly change and potentially creating havoc not to mention making it extremely difficult to troubleshoot.
As for memory usage, once the instance has no more references pointing to it, the garbage collector will get rid of it.
Two other notes on your code....
1) According to .NET patterns and practices, event arg classes should be named with "EventArgs" suffix. So your class name should be StatusEventArgs
2) Raising events should be always done in a subroutine named On<name of event>. In your case, your event should be named PartProcessed. You should then have a subroutine declared like this:
Protected Overridable OnPartProcessed(byval e as StatusEventArgs)
RaiseEvent PartProcessed(Me,e)
End Sub
Then instead of raising the event directly in your code, you call that sub.
This pattern allows inheriting classes to either override the event or insert code prior to the event being fired.
-
Thank you Archie, I see very well what you mean.
In my mind, I was seeing a new StatusArgs created every time the app verified a scan with potentially the exact same parameters as the last one that was created.
This particular event updates a picture box on the main form and sets the label for it.
This is just one of several things that I have looked at when trying to trim some fat in the app and optimize the code. My main concern is to try to resolve a high cpu usage on a Raspberry Pi 3.
Other areas were corrected that were constantly accessing properties of other classes, like inside a For Loop. Instead, I dim’d a local variable to first go grab the property, then use that variable in the For Loop.
It was also mentioned that the PictureBox control was frowned upon in regards to performance. An Image control was preferred unless features of the PictureBox were necessary.
-
If you are looking to optimize your code, Visual Studio has some very good tools. The Performance Analyzer in the Analyze menu can give some very good information, but can also be difficult to decipher. Essentially you run your program for a time period with the performance analyzer, then it can break down what code consumed the most CPU and memory.
-
If you are looking to optimize your code, Visual Studio has some very good tools. The Performance Analyzer in the Analyze menu can give some very good information, but can also be difficult to decipher. Essentially you run your program for a time period with the performance analyzer, then it can break down what code consumed the most CPU and memory.
Difficult to decipher. Yes. lol
Taking your advice, I ran the profiler and found that this bit of code is using the most memory, or 15%. Seems like using Resource Manager is quite taxing. I will try using direct file paths instead.
''' <summary>
''' Gets the image that the status image should be set to.
''' </summary>
''' <returns>Image</returns>
Public ReadOnly Property Image As Image
Get
Select Case _code
Case 0
Return Nothing
Case 1 'scanning complete
Return My.Resources.greenCheck
Case 2 'invalid scan
Return My.Resources.traffic_light_red
Case 3
Return My.Resources.important
Case 4 'station ready
Return My.Resources.traffic_light_green
Case Else
Return My.Resources.gear
End Select
End Get
End Property