49
votes

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?

8

8 Answers

40
votes

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.

66
votes

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.

7
votes

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.

5
votes

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

3
votes

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.

1
votes

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.

0
votes

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;
}
-1
votes

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);
}