Create delegate function on declared line, possible issues?

Question

If I have something like:

void SomeFunction(onCallBackDelegate)
{
     SomeClass.OnEvent += () => { onCallBackDelegate }
}

Will there be issues with functions being loaded up on top of one another every time SomeFunctioni s called? Is tehre a way to mitigate it (i.e. when the declared function is called it is removed from SomeClass.OnEvent)?

Just what language is that supposed to be? It's certainly not C. Tag the appropriate language (only), and present code in that language. — John Bollinger, Oct 03 '16 at 05:01
C has no delegates. You probably meant to use the C# tag instead. — kaylum, Oct 03 '16 at 05:11

vgru · Answer 1 · 2016-10-09T16:28:38.710

Besides not knowing if you attached the handler or not, the problem with anonymous handlers is that you cannot detach them from the event afterwards. With your specific example, if onCallBackDelegate has the same signature as the event delegate, there is no point in wrapping it in an anonymous method, just attach the delegate directly.

Regarding multiple subscription, depending on what the method actually does, your can either choose to register once (keep in mind this is not safe for multi-threaded access):

// client is making sure that the handler doesn't get 
// attached twice 
bool _callbackRegistered;
void SomeFunction(Action onCallBackDelegate)
{
     if (_callbackRegistered)
         return;

     _callbackRegistered = true;
     SomeClass.OnEvent += onCallBackDelegate;
}

Alternatively, you can check if event's invocation list already contains the delegate, but you can only do this inside the event's parent class, i.e.

class SomeClass
{
     private event Action SomeEvent;

     // the event is private, and we can only subscribe
     // through this method
     public void Subscribe(Action callback)
     {
         // 'callback' already attached?
         if (SomeEvent != null && SomeEvent.GetInvocationList().Contains(action))
             return;

         SomeEvent += callback;
     }

     public void Unsubscribe(Action callback)
     {
         SomeEvent -= callback;           
     }
}

One way also is to detach before you attach the handler, because detaching the handler won't throw an exception if the handler wasn't attached previously:

public void Subscribe(Action callback)
{
    // detach if already attached
    SomeEvent -= callback;

    // attach
    SomeEvent += callback;
}

The second pattern might provide a false sense of security, because if you keep attaching anonymous handlers, the Contains method will fail to detect a duplicate delegate. In other words:

void Handler() { }

// this will subscribe the handler only once:
someClass.Subscribe(Handler);
someClass.Subscribe(Handler);

// this will unsubscribe the handler and leave the invocation list empty
someClass.Unsubscribe(Handler);

// but this will subscribe **both** anonymous handlers:
someClass.Subscribe(() => Handler());
someClass.Subscribe(() => Handler());

// and this won't do anything    
someClass.Unsubscribe(() => Handler());

However, both of these patterns are, in my experience, most likely unnecessary. In most cases, you will instantiate a class (which provides some sort of a functionality) and attach to its handler there, only once:

// attach here and forget it
var someClass = new SomeClass();
someClass.OnEvent += Handler;

// once the someClass instance is no longer used, it will get
// collected and there is no need to detach your handler

score 0 · Answer 2 · edited May 23 '17 at 12:06

Every time you call SomeFunction(onCallBackDelegate) the onCallBackDelete will be added to the list of event subscribers. This in itself is not an issue, each registered onCallBackDelegate is called when the event fires. It depends on what onCallBackDelegate does whether or not this will give you problems in your code. There are many answers here on StackOverflow that show ways of preventing adding the same handler multiple times. See for instance here

Create delegate function on declared line, possible issues?

2 Answers2