C# adding and removing events from a timer

Question

I am trying to add and remove events from a timer and I have the following code:

Timer myTimer = new Timer(); // Windows.Forms Timer

public void addEvent(MyDelegate ev)
{
    myTimer.Tick += new EventHandler(ev);
}

public void removeEvent(MyDelegate ev)
{
    myTimer.Tick -= new EventHandler(ev);
}

I don't know If Im doing anything stupid in trying to add and remove delegates in this fashion, I am able to add delegates and get them to fire as expected. However, when I attempt to remove the events, they continue to fire on Timers Tick.

Can anyone see anything obviously wrong?

Answer 1

You should just be able to unsubscribe by referencing the name of your even handling method like so:

public void removeEvent(MyDelegate ev)
{
    myTimer.Tick -= ev as EventHandler;
}

Answer 2

When adding and removing event handlers you are creating a new wrapper for your delegate each time. So in your remove method it's trying to remove a new EventHandler object that was never added as a listener to the event in the first place.

If you want to keep using this type of setup, you could maybe stick your EventHandlers into a Dictionary. In the addEvent method, stick your newly created EventHandler into your dictionary, and in the removeEvent method, pull the EventHandler from the dictionary and remove that instead of instantiating a new one.

Answer 3

I believe that this code:

myTimer.Tick -= new EventHandler(ev);

creates a new EventHandler object. It will never remove an existing EventHandler. To get the functionality you want, you should be passing in EventHandlers, not MyDelegates, to the add and remove methods:

Timer myTimer = new Timer(); // Windows.Forms Timer

public void addEvent(EventHandler ev)
{
    myTimer.Tick += ev;
}

public void removeEvent(EventHandler ev)
{
    myTimer.Tick -= ev;
}

The calling code will have to keep track of the EventHandlers added, so that it can pass in the same EventHandler object when it is time to unsubscribe.

Answer 4

Your problem comes from having helper methods to do this. Without them, it works as expected, with them it does not know what to unhook.

To fix this, you will need to maintain a dictionary with the value being the EventHandler created in the hooking method so that you can remove that value later.

Something like:

var handlers = new Dictionary<MyDelegate, EventHandler>();

public void addEvent(MyDelegate ev)
{
    var handler = new EventHandler(ev);
    handlers.Add(ev, handler);
    myTimer.Tick += handler;
}

public void removeEvent(MyDelegate ev)
{
    myTimer.Tick -= handlers[ev];
}

You should add the appropriate checks if the element exists.

You could also change your parameter type and it will work as expected.

public void addEvent(EventHandler ev)
{
    myTimer.Tick += ev;
}

public void removeEvent(EventHandler ev)
{
    myTimer.Tick -= ev;
}

addEvent(new EventHandler(...));
removeEvent(new EventHandler(...));

Answer 5

I don't know what you're doing wrong, but the usual approach I would use for Timers would be to subscribe to the Tick event, and then disable the Timer when you don't want to receive the events, re-enabling when you do.

May not help you if you have more than one event handler hooked up to the event, but hopefully of some use.

Answer 6

The initial code works fine, as long as the MyDelegate 'ev' passed into addEvent and removeEvent is the same object instance (For example, if there is a class-level MyDelegate field that contains the instance or if you follow the advice of several others here and keep the MyDelegate object(s) in a Dictionary).

I suspect the problem is that the code calling addEvent and removeEvent is passing new MyDelegate instances pointing to some handler method, like so:

addEvent(new MyDelegate(this.HandlerMethod));
// ... do some stuff
removeEvent(new MyDelegate(this.HandlerMethod));

In which case addEvent and removeEvent are creating EventHandler delegates which point to different method addresses even though those delegates in turn are pointing to the same method (this.HandlerMethod). This is because the EventHandler delegates that add and remove create point to the MyDelegate.Invoke() method on different MyDelegate instances rather than directly to the address of this.HandlerMethod.