Using Delegates AND Declaring Events

Question

I'm developing a class library to be used for other developers and will be allowing them to either declare an instance of my class using WithEvents (or similar in other languages) as well as allow them to use Delegates defined in the class. Am I just being redundant here by doing it like this?

Public Delegate Sub TimerElapsedDelegate(ByVal sender As Object, ByVal e As System.EventArgs)
Public Event TimerElapsed(ByVal sender As Object, ByVal e As System.EventArgs)
Private _TimerElapsed As TimerElapsedDelegate = Nothing

Or should I just declare the events and let them do the AddHandler, etc., ?

Thanks for any advice on this ... I think I'm being redundant and don't want pointless code, not to mention avoiding the DRY principle.

{edit}Just wanted to post the remainder of the code, and stress that the "work" an instance of this class performs is done on a separate thread.{/edit}

#Region "Delegates"
Public Delegate Sub TimerElapsedDelegate(ByVal sender As Object, ByVal e As System.EventArgs)
Public Event TimerElapsed(ByVal sender As Object, ByVal e As System.EventArgs)
Private _TimerElapsed As TimerElapsedDelegate = Nothing
Public Property OnTimerElapsed() As TimerElapsedDelegate
    Get
        Return _TimerElapsed
    End Get
    Set(ByVal value As TimerElapsedDelegate)
        If value Is Nothing Then
            _TimerElapsed = Nothing
        Else
            If _TimerElapsed Is Nothing Then
                _TimerElapsed = value
            Else
                _TimerElapsed = System.Delegate.Combine(_TimerElapsed, value)
            End If
        End If
    End Set
End Property
Private Sub TriggerTimerElapsed()
    If OnTimerElapsed IsNot Nothing Then
        OnTimerElapsed.Invoke(Me, New System.EventArgs)
    End If
    RaiseEvent TimerElapsed(Me, New System.EventArgs)
End Sub

Public Delegate Sub ItemReadyForQueueDelegate(ByVal sender As Object, ByVal e As System.EventArgs)
Public Event ItemReadyForQueue(ByVal sender As Object, ByVal e As System.EventArgs)
Private _ItemReadyForQueue As ItemReadyForQueueDelegate = Nothing
Public Property OnItemReadyForQueue() As ItemReadyForQueueDelegate
    Get
        Return _ItemReadyForQueue
    End Get
    Set(ByVal value As ItemReadyForQueueDelegate)
        If value Is Nothing Then
            _ItemReadyForQueue = Nothing
        Else
            If _ItemReadyForQueue Is Nothing Then
                _ItemReadyForQueue = value
            Else
                _ItemReadyForQueue = System.Delegate.Combine(_ItemReadyForQueue, value)
            End If
        End If
    End Set
End Property
Private Sub TriggerItemReadyForQueue(ByVal oItem As h3Budgeteer.FileSystem.ReportTemplateFile.ReportTemplate)
    If OnItemReadyForQueue IsNot Nothing Then
        OnItemReadyForQueue.Invoke(Me, New ItemReadyForQueueEventArgs(oItem))
    End If
    RaiseEvent ItemReadyForQueue(Me, New ItemReadyForQueueEventArgs(oItem))
End Sub
Public Class ItemReadyForQueueEventArgs
    Inherits System.EventArgs
    Private _ReportTemplate As h3Budgeteer.FileSystem.ReportTemplateFile.ReportTemplate = Nothing
    Public ReadOnly Property ReportTemplate() As h3Budgeteer.FileSystem.ReportTemplateFile.ReportTemplate
        Get
            Return _ReportTemplate
        End Get
    End Property
    Public Sub New(ByVal oReportTemplate As h3Budgeteer.FileSystem.ReportTemplateFile.ReportTemplate)
        _ReportTemplate = oReportTemplate
    End Sub
End Class

End Region

Answer 1

I would say just completely remove your delegate entirely.

Your delegate is doing exactly the same thing as the event. You are pretty much writing your own event plumbing instead of using the framework's Event call. An Event is pretty much exactly what you've written, except that it's easier to use, and also makes it easier to unsubscribe from the event.

There is no advantage to providing both - The event does everything that your "delegate" does, and is much more clear.

(Previously:)

If you're developing this as a class library, I would suggest just making your class not be sealed, and following the more standard approach. The normal approach for allowing logic to be overridden or inserted into your code and allowing events would be to provide hooks for subclassing.

Delegates could be used in a situation like this to allow the user to plug in their own logic. However, in many cases, having protected virtual functions makes this more clear, and much easier to accomplish.

Events should be exactly that, an event that notifies the user of some "event". These should be hooks where the user attaches their delegate.

For example, instead of providing delegates and events, the base Windows Forms controls use a protected method (ie: OnMouseDown) and an event that's triggered by default (MouseDown).

This allows a user to subclass your class and override the logic (which is probably why you'd want delegates) as well as handle the event.

The one place where I would provide delegates is in rare cases where your class or method REQUIRES logic to be added by a user. In this case, you can either provide an abstract base class, or have a delegate that is passed in for that logic. A good example of this is the .Where() method in LINQ. Where is useless without the predicate used for filtering, so passing in a delegate makes sense in this case. Note, though, that there is no event associated with this - it's really there to provide a different function.

Answer 2

You can get rid of the delegate by using a generic EventHandler. All you have to do is create your own class that inherits from EventArgs.

Public Class Foo
    Inherits EventArgs
End Class

Public Class Bar
    Public Event MyEvent As EventHandler(Of Foo)
End Class

I don't think you're being redundant. See the first answer to this question. Adding an empty event handler will guarantee people using your event will not get a NullReferenceException when it's fired if they don't want to listen to / handle the event.

-EDIT-

After seeing your code, I agree with Reed. Since it's going to be a shared library, I don't think that you need to execute the consumer's event handler. The job of your library is to just fire off the event and let the consumer know something happened. It's up to them to handle or not handle the event.

I would say that your properties are redundant. They are, in essence, event handlers.

Answer 3

For your class library all you need is to write the public Event line of code.

Public Event TimerElapsed(ByVal sender As Object, ByVal e As System.EventArgs)

Make sure to raise the event anywhere in your library of course. Any client developers would be the ones to add a handler to the event.

Its not redundant, its just not necessary unless your library is going to handle any events from that class.