3

I have some C# code that updates some properties of an object. I have an event handler defined to help me respond when the update process is done. Unfortunately, I've learned that this event is getting fired multiple times. I suspect this is happening because the event handler is being set at the wrong time. Currently, it is being set like the following:

myObject.Update_Succeeded += new EventHandler(myObject_Update_Succeeded);

Due to the complexity of the code, I'm having a difficult time of tracking down where it should be set. I would like to only set the event handler it hasn't been previously set. Because of this, I want to do something like this:

ClearEventHandlers(myObject);

or

myObject.Update_Succeeded = null;
myObject.Update_Succeeded += new EventHandler(myObject_Update_Succeeded);

Is there a way to accomplish what I'm trying?

Thank you!

user609886
  • 1,639
  • 4
  • 25
  • 38

4 Answers4

6

Yes, you can customize the add/remove accessors of your event. This article describes these accessors. But you can do something like:

class MyClass
{
    private EventHandler _myEvent;

    public event EventHandler MyEvent
    {
        [MethodImpl(MethodImplOptions.Synchronized)]
        add 
        { 
            _myEvent = (EventHandler)Delegate.Combine(_myEvent, value);
        }
        [MethodImpl(MethodImplOptions.Synchronized)]
        remove 
        { 
            _myEvent = (EventHandler)Delegate.Remove(_myEvent, value); 
        }
    }

    public void ClearMyEvent() {
        _myEvent = null;
    }
    ...
}
CodeNaked
  • 40,753
  • 6
  • 122
  • 148
  • I wouldn't recommend such approach. Event listeners should not have the freedom to unsubscribe other event listeners they know nothing about. – vgru Aug 02 '11 at 12:07
  • @Groo - In general, I'd agree. But there could be times where there needs to be a way to clear all listeners. – CodeNaked Aug 02 '11 at 12:09
  • I must confess I haven't been in such situation yet. And there is not a single class in BCL which follows the pattern you described (nor in any open source libraries I had the chance to look at so far). – vgru Aug 02 '11 at 12:14
  • 1
    @Groo - Neither have I, but that doesn't mean it would never need to be done :-) – CodeNaked Aug 02 '11 at 12:24
  • I need to do this for game objects that are recycled when destroyed. Without being able to externally clear the event list, I have to make and use a minimum of two events for every object - one for the event of interest, and another just to be notified when the object is destroyed so that it can remove it's event handlers from various event chains. This extra event handling costs performance and doesn't add any benefit over resetting it externally as shown above. This pattern isn't good for everything, but for recycling situations where your code is set up for this, it can make sense to do. – Eric Cosky Jan 29 '12 at 19:39
4

You should be able to remove a handler using the subtract operator like below

myObject.Update_Succeeded -= new EventHandler(myObject_Update_Succeeded);

Or check this out for a way to remove all event handler if you are in doubt

How to remove all event handlers from a control

Community
  • 1
  • 1
Fadrian Sudaman
  • 6,405
  • 21
  • 29
2

Proper way should be to detach the handler from each event after you no longer use it:

public class MyObjectListener
{
     private readonly MyObject _object;
     public class MyObjectListener(MyObject obj)
     {
          _object = obj;
          Attach();
     }

     // adds event handlers
     private void Attach()
     {
         obj.UpdateSucceeded += UpdateSuceededHandler;
         obj.UpdateFailed += UpdateFailedHandler;
     }

     // removes event handlers
     private void Detach()
     {
         obj.UpdateSucceeded -= UpdateSuceededHandler;
         obj.UpdateFailed -= UpdateFailedHandler;
     }

     ...
}

The only thing you need to decide is where to call the Detach method. For example, you can call it in the handler itself:

     private void UpdateSuceededHandler(object sender, Data args)
     {
         Detach();
         // do something when it succeeds
     }

     private void UpdateFailedHandler(object sender, Data args)
     {
         Detach();
         // do something when it fails
     }

Or, you could allow users of MyObjectListener to tell it that it no longer needs to listen to the attached object:

     public void StopListening()
     {
         Detach();
     }

An object which raises an event should not allow its listeners to modify the event invocation list. Each event listener should subscribe or unsubscribe its own event handlers only.

vgru
  • 49,838
  • 16
  • 120
  • 201
0

You better set event handler on the initialization of your object i.e. in your Constructor.

Arief
  • 6,055
  • 7
  • 37
  • 41