52

I would like to ensure that I only subscribe once in a particular class for an event on an instance.

For example I would like to be able to do the following:

if (*not already subscribed*)
{
    member.Event += new MemeberClass.Delegate(handler);
}

How would I go about implementing such a guard?

Glen T
  • 1,550
  • 1
  • 13
  • 22

8 Answers8

75

I'm adding this in all the duplicate questions, just for the record. This pattern worked for me:

myClass.MyEvent -= MyHandler;
myClass.MyEvent += MyHandler;

Note that doing this every time you register your handler will ensure that your handler is registered only once.

alf
  • 18,372
  • 10
  • 61
  • 92
  • 5
    Works for me too, and this looks to me like the best solution for cases where you don't have access to the class (in my case Form.KeyDown) – M-Peror Jan 17 '12 at 10:28
  • 9
    there is a caveat to this solution. If you are unsubscribed at a moment when an event comes in you'll miss the event so make sure 100% no events are coming in between the unsubscription and subscription. – Denis Nov 14 '16 at 15:38
  • In my case, were I was trying to avoid WPF control event duplication this solution was the easiest and the cleanest way. – Evgeniy Miroshnichenko Jan 08 '18 at 19:15
  • It will seems obvious for most of us, but I would like to precise something : it works only if you unsubscribe/subscribe for the SAME INSTANCE of a class. It won't work for different instances, unlike "_eventHasSubscribers" approach of other answers for example. – AFract Sep 04 '19 at 09:38
  • This didn't work for me, and I think it's because of multithreading. The "MyHandler" wasn't the same object as previous versions of the event, so it was never removed and was added again and again etc. I had to verify that the MyHandler.Method wasn't already added before adding the MyHandler event. – computercarguy Jul 30 '20 at 18:45
41

If you are talking about an event on a class that you have access to the source for then you could place the guard in the event definition.

private bool _eventHasSubscribers = false;
private EventHandler<MyDelegateType> _myEvent;

public event EventHandler<MyDelegateType> MyEvent
{
   add 
   {
      if (_myEvent == null)
      {
         _myEvent += value;
      }
   }
   remove
   {
      _myEvent -= value;
   }
}

That would ensure that only one subscriber can subscribe to the event on this instance of the class that provides the event.

EDIT please see comments about why the above code is a bad idea and not thread safe.

If your problem is that a single instance of the client is subscribing more than once (and you need multiple subscribers) then the client code is going to need to handle that. So replace

not already subscribed

with a bool member of the client class that gets set when you subscribe for the event the first time.

Edit (after accepted): Based on the comment from @Glen T (the submitter of the question) the code for the accepted solution he went with is in the client class:

if (alreadySubscribedFlag)
{
    member.Event += new MemeberClass.Delegate(handler);
}

Where alreadySubscribedFlag is a member variable in the client class that tracks first subscription to the specific event. People looking at the first code snippet here, please take note of @Rune's comment - it is not a good idea to change the behavior of subscribing to an event in a non-obvious way.

EDIT 31/7/2009: Please see comments from @Sam Saffron. As I already stated and Sam agrees the first method presented here is not a sensible way to modify the behavior of the event subscription. The consumers of the class need to know about its internal implementation to understand its behavior. Not very nice.
@Sam Saffron also comments about thread safety. I'm assuming that he is referring to the possible race condition where two subscribers (close to) simultaneously attempt to subscribe and they may both end up subscribing. A lock could be used to improve this. If you are planning to change the way event subscription works then I advise that you read about how to make the subscription add/remove properties thread safe.

Community
  • 1
  • 1
Hamish Smith
  • 8,153
  • 1
  • 34
  • 48
  • I think I'll go ahead with the boolean member variable approach. However I'm a little surprised that there isn't another way to check if a client is already subscribed. I would have thought it was more common for a given client to only want to subscribe once? – Glen T Dec 15 '08 at 07:26
  • 1
    Depending on your setup you may want to throw an exception if the event already has a subscriber. If you are adding subscribers at run time this will notify them of the error instead of doing nothing. Changing the default behaviour wihtout notifying the user isn't the best practice. – Rune Grimstad Dec 15 '08 at 07:58
  • 3
    Also this is a major anti-pattern, arbitrary consumers need to be privy to your event implementation to understand its behavior. – Sam Saffron Jul 07 '09 at 02:15
  • 1
    @Sam Saffron: thanks for the comments. Have attempted to place some (further) warnings about the first method into the answer. – Hamish Smith Jul 30 '09 at 19:22
7

As others have shown, you can override the add/remove properties of the event. Alternatively, you may want to ditch the event and simply have the class take a delegate as an argument in its constructor (or some other method), and instead of firing the event, call the supplied delegate.

Events imply that anyone can subscribe to them, whereas a delegate is one method you can pass to the class. Will probably be less surprising to the user of your library then, if you only use events when you actually want the one-to-many semantics it usually offers.

Pang
  • 9,564
  • 146
  • 81
  • 122
jalf
  • 243,077
  • 51
  • 345
  • 550
5

You can use Postsharper to write one attribute just once and use it on normal Events. Reuse the code. Code sample is given below.

[Serializable]
public class PreventEventHookedTwiceAttribute: EventInterceptionAspect
{
    private readonly object _lockObject = new object();
    readonly List<Delegate> _delegates = new List<Delegate>();

    public override void OnAddHandler(EventInterceptionArgs args)
    {
        lock(_lockObject)
        {
            if(!_delegates.Contains(args.Handler))
            {
                _delegates.Add(args.Handler);
                args.ProceedAddHandler();
            }
        }
    }

    public override void OnRemoveHandler(EventInterceptionArgs args)
    {
        lock(_lockObject)
        {
            if(_delegates.Contains(args.Handler))
            {
                _delegates.Remove(args.Handler);
                args.ProceedRemoveHandler();
            }
        }
    }
}

Just use it like this.

[PreventEventHookedTwice]
public static event Action<string> GoodEvent;

For details, look at Implement Postsharp EventInterceptionAspect to prevent an event Handler hooked twice

Pang
  • 9,564
  • 146
  • 81
  • 122
Saghar
  • 693
  • 2
  • 12
  • 24
3

You would either need to store a separate flag indicating whether or not you'd subscribed or, if you have control over MemberClass, provide implementations of the add and remove methods for the event:

class MemberClass
{
        private EventHandler _event;

        public event EventHandler Event
        {
            add
            {
                if( /* handler not already added */ )
                {
                    _event+= value;
                }
            }
            remove
            {
                _event-= value;
            }
        }
}

To decide whether or not the handler has been added you'll need to compare the Delegates returned from GetInvocationList() on both _event and value.

Andrew Kennan
  • 13,947
  • 3
  • 24
  • 33
2

I know this is an old Question, but the current Answers didn't work for me.

Looking at C# pattern to prevent an event handler hooked twice (labelled as a duplicate of this question), gives Answers that are closer, but still didn't work, possibly because of multi-threading causing the new event object to be different or maybe because I was using a custom event class. I ended up with a similar solution to the accepted Answer to the above Question.

private EventHandler<bar> foo;
public event EventHandler<bar> Foo
{
    add
    {
        if (foo == null || 
            !foo.GetInvocationList().Select(il => il.Method).Contains(value.Method))
        {
            foo += value;
        }
    }

    remove
    {
        if (foo != null)
        {
            EventHandler<bar> eventMethod = (EventHandler<bar>)foo .GetInvocationList().FirstOrDefault(il => il.Method == value.Method);

            if (eventMethod != null)
            {
                foo -= eventMethod;
            }
        }
    }
}

With this, you'll also have to fire your event with foo.Invoke(...) instead of Foo.Invoke(...). You'll also need to include System.Linq, if you aren't already using it.

This solution isn't exactly pretty, but it works.

computercarguy
  • 2,173
  • 1
  • 13
  • 27
0

I did this recently and I'll just drop it here so it stays:

private bool subscribed;

if(!subscribed)
{
    myClass.MyEvent += MyHandler;
    subscribed = true;
} 

private void MyHandler()
{
    // Do stuff
    myClass.MyEvent -= MyHandler;
    subscribed = false;
}
Tony Steel
  • 123
  • 1
  • 6
-1

Invoke only distinct elements from GetInvocationList while raising:

using System.Linq;
....
public event HandlerType SomeEvent;
....
//Raising code
foreach (HandlerType d in (SomeEvent?.GetInvocationList().Distinct() ?? Enumerable.Empty<Delegate>()).ToArray())
     d.Invoke(sender, arg);

Example unit test:

class CA 
{
    public CA()
    { }
    public void Inc()
        => count++;
    public int count;
}
[TestMethod]
public void TestDistinctDelegates()
{
    var a = new CA();
    Action d0 = () => a.Inc();
    var d = d0;
    d += () => a.Inc();
    d += d0;
    d.Invoke();
    Assert.AreEqual(3, a.count);
    var l = d.GetInvocationList();
    Assert.AreEqual(3, l.Length);
    var distinct = l.Distinct().ToArray();
    Assert.AreEqual(2, distinct.Length);
    foreach (Action di in distinct)
        di.Invoke();
    Assert.AreEqual(3 + distinct.Length, a.count);
}
[TestMethod]
public void TestDistinctDelegates2()
{
    var a = new CA();
    Action d = a.Inc;
    d += a.Inc;
    d.Invoke();
    Assert.AreEqual(2, a.count);
    var distinct = d.GetInvocationList().Distinct().ToArray();
    Assert.AreEqual(1, distinct.Length);
    foreach (Action di in distinct)
        di.Invoke();
    Assert.AreEqual(3, a.count);
}
Кое Кто
  • 445
  • 5
  • 9