Avoid duplicate event subscriptions in C#

Question

How would you suggest the best way of avoiding duplicate event subscriptions? if this line of code executes in two places, the event will get ran twice. I'm trying to avoid 3rd party events from subscribing twice.

theOBject.TheEvent += RunMyCode;

In my delegate setter, I can effectively run this ...

theOBject.TheEvent -= RunMyCode;
theOBject.TheEvent += RunMyCode;

but is that the best way?

Answer 1

I think, the most efficient way, is to make your event a property and add concurrency locks to it as in this Example:

private EventHandler _theEvent;
private object _eventLock = new object();
public event EventHandler TheEvent
{
    add
    {
        lock (_eventLock) 
        { 
            _theEvent -= value; 
            _theEvent += value; 
        }
    }
    remove
    {
        lock (_eventLock) 
        { 
           _theEvent -= value; 
        }
    }
}

Answer 2

I have done this before....it assumes it is acceptable that the last subscriber is what gets called.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace ConsoleApplication2
{
    class Program
    {
        static void Main(string[] args)
        {
            MyObject my = new MyObject();
            my.Changed += new EventHandler(my_Changed);
            my.Changed += new EventHandler(my_Changed1);

            my.Update();
            Console.ReadLine();
        }

        static void my_Changed(object sender, EventArgs e)
        {
            Console.WriteLine("Hello");
        }
        static void my_Changed1(object sender, EventArgs e)
        {
            Console.WriteLine("Hello1");
        }
    }
    public class MyObject
    {
        public MyObject()
        {
        }
        private EventHandler ChangedEventHandler;
        public event EventHandler Changed
        {
            add
            {
                ChangedEventHandler = value;
            }
            remove
            {
                ChangedEventHandler -= value;
            }
        }
        public void Update()
        {
            OnChanged();
        }

        private void OnChanged()
        {
            if (ChangedEventHandler != null)
            {
                ChangedEventHandler(this, null);
            }
        }
    }
}

Answer 3

Is your code multi threaded ? Concurrency lock is needed only when its multi threaded. If not its a overhead.

As such your approach of unsubscribing and subscribing is correct.

Thanks

Answer 4

If you own the source for the class of theObject, then you have access to the InvocationList of TheEvent. You can implement your own add accessor for the event and check before adding.

However, I think that your approach is fine too.

Answer 5

I use your approach except one detail. I think, that events should be subscribed when you create new instance of subscriber or theObject, this makes code more straight. Thus, all you need is just carefully watch after correct objects disposing (dispose patten is convenient solution for this).

You mentioned that you use 3rd party event, that means that you can't provide your own realisation for add/remove methods, as you have been advised. But in your own classes with your own events you should define your own realisation of add/remove methods for event in order to solve your problem.