Another way to implement weak event handlers in vb.net

Summary: Please let me know, what weaknesses do you see in the described workaround for event handlers preventing garbage collection of registered listener objects...

I'm in the development of and API for a set of devices communicating with each other, and optionally with a PC trough a socket based gateway. The API describes the multitude of different devices, channels as a class hierarchy, and also implements some kind of caching for the derived objects.

I'd like to make the API really simple to use for the developers, so I keep the interfaces as simple as possible: hiding background threading, etc... I ran into the already described problems, when registering event handlers from a windows form interface:

-
The event source object is in the cache, it keeps the reference to the target object, preventing it from garbage collection. It's quite problematic, because the UI creates and destroys controls (providing some kind of user interface for the device objects) on the fly... for example when a new device is discovered, or when view is changed.

-
As the event raiser might be a different thread than the UI, so it requires checking in each of the event handler routines.

I was reading the WeakEvent pattern guidelines - I'm not too happy with that solution:

• It's not "standard" - so the programmer has to register for events "manually"
• And quite complex to build.

I ended up with Custom Event declarations, like:

Private EHL_Received As New PacketEventHandlerList("PacketReceived")
Public Custom Event PacketReceived As EventHandler Implements I_PacketEventRaiser.PacketReceived
AddHandler(ByVal value As EventHandler)
EHL_Received.Add(value)
End AddHandler
RemoveHandler(ByVal value As EventHandler)
EHL_Received.Remove(value)
End RemoveHandler
RaiseEvent(ByVal sender As Object, ByVal e As PacketEventArgs)
EHL_Received.Invoke(sender, e)
End RaiseEvent
End Event


Then, in the "event handler" list, instead of stor

At first glance, PacketEventHandler is a rather confusing name for a class, but then looking at it more thoroughly I see that it's actually a wrapper around an event handler, so I see what made you call it that way, but I'd still try harder to find a better name... although I have none to suggest right now (naming is one of the hardest things to do properly!).

The way you're clearing your InvalidHandlers looks awkward.

' If there were invalid handlers, just clean them up
If InvalidHandlers > 0 Then
    For I As Integer = Handlers.Count - 1 To 0 Step -1
        If Not Handlers(I).IsValid Then Handlers.RemoveAt(I)
    Next
End If

Handlers being a List(Of PacketEventHandler), I'm pretty sure you could simply do this:

Dim Handler As PacketEventHandler
If InvalidHandlers > 0 Then
    For Each Handler In Handlers
        If Not Handler.IsValid Then Handlers.Remove(Handler)
    Next
End If

And reviewing this snippet made me wonder, are you using Option Explicit? I don't see your I declared anywhere - always use Option Explicit. The C# programmer in me would also advise you to use Option Strict just to be on a [type-]safe side (you don't have to specify explicit if you specify strict - the latter implies the former).

This is by no means a full review - but it's all I could see on a first pass. The rest roughly looks good to me.

StackExchange Code Review Q#8776, answer score: 3