Eventhandler tips

You've implemented the Observer pattern.
You don't need to do that any more in .NET

First of all, you don't need to add the handler to your timer each time you start/stop the timer.
Add the handler at your constructor of your EventHandler class once.
If the timer does not run, it will not fire.

If you create a public event in your EventHandler class, your subscribers can subscribe themselves easiliy be calling e.g.
AddHandler EventHandler.SomethingHappened, AddressOf MyHandler

The framework will take care of notifying all subscribers of the event when the SomethingHappened event gets raised.

thx for the feedback, some questions..

What is the Observer pattern? and what should I change to remove that pattern..

Well, the Observer pattern is the pattern you have implemented.
It's a design pattern.
http://www.dofactory.com/Patterns/PatternObserver.aspx

You have to remove the Register part. Registering for events is handled by .NET when you do the AddHandler stuff.
Probably underwater in the .NET Framework the Observer pattern will be used to translate the AddHandler again :D

oki, but how do I fire the GenerateEvent function if I don't register the objects

And one other thing is that the event's must not fire before the timer says it could.

Can I keep this even if I remove the opserver pattern.,

I'm reading this now..
http://msdn.microsoft.com/en-us/library/ms998543.aspx

Is it that you want me to change to.. delegates?

well, in VB.NET you don't have to.

you can create an Event

public Event TimerElapsed(blabla as string)

in your code:
Raisevent TimerElapsed("sdfsdf")

Didn't know there was an article explaining exactly what I was trying to tell you :D
Example is C# though, in VB.NET you will implement it with the Event type.

ok, i'm trying to remove the observer pattern now.. If I use the example in the article where shoud I implement the timer functions? In each object (Album class) or in the Main form class that handels all the objects?

Im' not sure where to implement all the difrent functions I have.

oki. I got it to work now.. I think

The thing is that I need to start the event stuff for ech object now instead of just adding all objects to the handler and then runing them all with one function.

and of course with this I get more control of which objects that should be executed-

Can you please check the code and see if I have done it right? :)

Imports System
Imports System.Timers
Imports System.Collections
 
Public Class Album
    Private m_name As String
    Private m_Timer As New System.Timers.Timer
    Private dFrequency As Double
 
    Public Delegate Sub PlayHandler(ByVal sender As Object)
    Public Delegate Sub FireHandler(ByVal sender As Object)
    Public Event PlayEvent As PlayHandler
    Public Event FireEvents As FireHandler
 
    Public Sub New(ByVal name As String, ByVal Frequency As Double)
        Me.m_name = name
        dFrequency = Frequency
    End Sub
 
    Public Sub Play()
        Notify()
 
        ' code to play the album 
    End Sub
 
    Private Sub Notify()
        RaiseEvent PlayEvent(Me)
    End Sub
 
    Public ReadOnly Property Name() As String
        Get
            Return m_name
        End Get
    End Property
 
    '// Start the events
    Public Sub StartEvents()
 
        '// Start the timer if it's not already running
        If Not m_Timer.Enabled Then
            '// Set the Interval(milliseconds).
            Dim milliseconds As Integer = (1 / dFrequency) * 1000
 
            '// Add the Elapsed event for the timer.
            AddHandler m_Timer.Elapsed, AddressOf OnTimedEvent
 
            m_Timer.Interval = milliseconds
            m_Timer.Enabled = True
            m_Timer.Start()
            ' Keep the timer alive until the end of Main.
            'GC.KeepAlive(aTimer)
        End If
    End Sub
 
 
    '// Stop the events
    Public Sub StopEvents()
 
        '// Start the timer if it's not already running
        If m_Timer.Enabled Then
            '// Disable timer
            m_Timer.Enabled = False
 
            '// Remove the Elapsed event for the timer.
            RemoveHandler m_Timer.Elapsed, AddressOf OnTimedEvent
        End If
    End Sub
 
    ' Specify what you want to happen when the Elapsed event is 
    ' raised.
    Private Sub OnTimedEvent(ByVal source As Object, ByVal e As ElapsedEventArgs)
        RaiseEvent FireEvents(Me)
    End Sub
 
End Class
 
Public Class Form1
    Private Event OnChange()
    Public Delegate Sub UpdateTextCallback(ByVal text As String)
 
    Private Sub Form1_Load(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles MyBase.Load
        Dim billing As New BillingService()
        Dim album As New Album("Up", 1)
 
        m_TextBox = ListBox1
 
        AddHandler album.FireEvents, AddressOf UpdateForm
        album.Play()
        album.StartEvents()
 
    End Sub
 
    Public Sub GenerateEvents()
 
        If True Then
            RaiseEvent OnChange()
        End If
    End Sub
 
    Private Sub UpdateForm(ByVal subject As Object)
        If TypeOf subject Is Album Then
            GenerateEvents()
        End If
    End Sub
 
    Private Sub GetData() Handles Me.OnChange
        'ListView1.Items.Add("Event was fired")
        m_TextBox.Invoke(New UpdateTextCallback(AddressOf UpdateText), New Object() {"Text generated on non-UI thread."})
 
    End Sub
 
    Private m_TextBox As ListBox
    ' The declaration of the textbox. 
 
    ' Updates the textbox text. 
    Private Sub UpdateText(ByVal text As String)
        ' Set the textbox text. 
        m_TextBox.Items.Add(text)
    End Sub
 
End Class

Select allOpen in new window

yeah, you could do something like that.

If you need more information about the event you could introduce eventargs as well.

e.g.
public class AlbumEventArgs
inherits EventArgs
   
 ' do what ever you want to do in here.
  ' most of the time you want to have readonly properties here, that you only can set via the constructor.
 
end class

It's a good practise to have sender and eventargs in your event.

Ok now the eventhandler has been redesigned, Have you some time to help me look at tis design?

I can create a new question if it's better.?

creating a new question is up to you...
post your code and I'll have a look

Here is the code... I can't get it to work. You will see what I wan't.

This structure benefits the application. I have tested the other approch you showed me but in this specific case it will work better with this structure.  I have several client/server application that will be communicating with each other and bacause how they comunicate this approach should work..
I hope.. hehe

So If you check the code you can see the i'm trying to reach the Proxy object but I don't know how I will do that without making a new instance of that object. And I don't want to do that.

You can change anything if you find that the design is faulty.

Imports System
Imports System.Timers
Imports System.Collections
 
Public Class GetData
    Inherits ObserverEventhandler
 
    Private Shared m_Timer As New System.Timers.Timer
    Public Event OnChange(ByVal m_data As Object)
 
    Public Sub New(ByVal Sender As Object, ByVal Proxy1 As Object, ByVal Proxy2 As Object)
        MyBase.New(Sender, Proxy1, Proxy2)
    End Sub
 
    ''' <summary>
    ''' Start fire the OnTimedEvent with specifed freqency
    ''' </summary>
    Public Shared Sub StartEvents(ByVal Frequency As Double)
        Try
            '// Start the timer if it's not already running
            If Not m_Timer.Enabled Then
                '// Set the Interval(milliseconds)
                Dim milliseconds As Integer = CInt((1 / Frequency) * 1000.0)
 
                '// Add the Elapsed event for the timer.
                AddHandler m_Timer.Elapsed, AddressOf OnTimedEvent
 
                m_Timer.Interval = milliseconds
                m_Timer.Enabled = True
                m_Timer.Start()
            End If
        Catch ex As Exception
            Throw
        End Try
    End Sub
 
    ''' <summary>
    ''' Stop rolling the events
    ''' </summary>
    Public Shared Sub StopEvents()
        Try
            '// Start the timer if it's not already running
            If m_Timer.Enabled Then
                '// Disable timer
                m_Timer.Enabled = False
 
                '// Remove the Elapsed event for the timer.
                RemoveHandler m_Timer.Elapsed, AddressOf OnTimedEvent
            End If
        Catch ex As Exception
            Throw
        End Try
    End Sub
 
    ''' <summary>
    ''' Fire GenererateEvents sub in each added object when the the timer event is raised
    ''' </summary>
    Private Shared Sub OnTimedEvent(ByVal source As Object, ByVal e As ElapsedEventArgs)
        Try
            For Each obj As Object In m_AllInstances
                obj.[GenerateEvents]()
            Next
            If Proxy1 IsNot Nothing Then
                If Proxy1.[Changed] Then
                    RaiseEvent OnChange(Prox1.[ReadData])
                End If
            End If
 
            If Proxy2 IsNot Nothing Then
                If Proxy2.[Changed] Then
                    RaiseEvent OnChange(Prox1.[ReadData])
                End If
            End If
        Catch ex As Exception
            Throw
        End Try
    End Sub
End Class
 
Imports System
Imports System.Timers
Imports System.Collections
 
Public MustInherit Class ObserverEventhandler
    Private Shared m_AllInstances As Collection
    Protected Friend Proxy1 As Object = Nothing
    Protected Friend Proxy2 As Object = Nothing
    Protected Friend Sender As Object = Nothing
 
    Friend Sub New(ByVal Sender As Object, ByVal Proxy1 As Object, Optional ByVal Proxy2 As Object = Nothing)
        MyBase.New()
 
        m_AllInstances = New Collection '// Create a new collection object
        Sender = Sender               '// The sender, the object that controls the proxy objects
        Proxy1 = Proxy1               '// Proxy object 1
        Proxy1 = Proxy2               '// Optinal proxy object 2
        [m_AllInstances].Add(Sender)
 
    End Sub

Select allOpen in new window

You might want to introduce an interface for your proxy object:
Public Interface IProxy
    Property IsChanged() As Boolean
    Function ReadData() As Object

    Event OnChange(ByVal m_data As Object)
End Interface

Then you can change the type of Proxy1 and Proxy2 from Object to IProxy.
In that way you can access its properties/methods in a better way.

You expect the sender to have a Method called GenerateEvents as well:

Public Interface IEventObject
    Sub GenerateEvents()
End Interface

Change the type of sender to IEventObject.

Note that I've added the Me.Proxy1. Otherwise nothing gets assigned.

Option Strict On
Option Explicit On
 
Imports System
Imports System.Timers
Imports System.Collections
 
Public MustInherit Class ObserverEventhandler
    Protected Shared AllInstances As List(Of IEventObject)
    Protected Friend Proxy1 As IProxy = Nothing
    Protected Friend Proxy2 As IProxy = Nothing
    Protected Friend Sender As IEventObject = Nothing
 
    Friend Sub New(ByVal sender As IEventObject, ByVal Proxy1 As IProxy, Optional ByVal Proxy2 As IProxy = Nothing)
        MyBase.New()
 
        AllInstances = New List(Of IEventObject) '// Create a new collection object
        Me.Sender = sender               '// The sender, the object that controls the proxy objects
        Me.Proxy1 = Proxy1               '// Proxy object 1
        Me.Proxy2 = Proxy2               '// Optional proxy object 2
        AllInstances.Add(sender)
 
    End Sub
 
End Class

Select allOpen in new window

Thx a lot m8, I will check the code and get back to you.. :)

One question..

How should this sub look then if I chnage to the interface

    Private Shared Sub OnTimedEvent(ByVal source As Object, ByVal e As ElapsedEventArgs)
        Try
            If Proxy1 IsNot Nothing Then
                If Proxy1.[Changed] Then
                    RaiseEvent OnChange(Prox1.[ReadData])
                End If
            End If
 
            If Proxy2 IsNot Nothing Then
                If Proxy2.[Changed] Then
                    RaiseEvent OnChange(Prox1.[ReadData])
                End If
            End If
        Catch ex As Exception
            Throw
        End Try
    End Sub

Select allOpen in new window

oki, you have now removed the shared =>  the proxy1 will be reached but then I will have some problem to run the StartEvents function because that function is Shared.

or chan I change the

 Protected Friend Proxy1 As IProxy = Nothing

to

 Protected Friend Shared Proxy1 As IProxy = Nothing


The main problem is that StartEvents is shared.. but I would like to keep it shared. don't want to make an instance of getdata class

oh, yeah, you can make it shared, normally I don't like to make everything shared, but in your case it could suit your needs.

one question.. if I do
Protected Friend Share

is that corrrect? the class the declaration exist in is a mustinherit class is Friend doing any good here?

well, that depends. do you want classes outside of your assembly being able to inherit from your class? if so, don't use friend

I will test the code and get back to you.. don't want to let you go just yet. I can see that you know what you are talking about. Nice to get some input from an expert. .)

I'm here to help...