How do I Unregister 'anonymous' event handler [duplicate]
Asked Answered
U

7

38

Say if I listen for an event:

Subject.NewEvent += delegate(object sender, NewEventArgs e)
{
    //some code
}); 

Now how do I un-register this event? Or just allow the memory to leak?

Unscrupulous answered 28/8, 2009 at 16:35 Comment(1)
see here #183867Eastlake
W
25

If you need to unregister an event, I recommend avoiding anonymous delegates for the event handler.

This is one case where assigning this to a local method is better - you can unsubscribe from the event cleanly.

Woodrowwoodruff answered 28/8, 2009 at 16:38 Comment(7)
I thought so. But, had a doubt. Thanks.Unscrupulous
I disagree - if you need to create a closure then you are going to have to use an anonymous method.Tangerine
@free-dom: There are always options to avoid making closures (worst case, you could do what the compiler does for you). Most of the time, event handlers where you plan to unsubscribe the event are not, IMO, good candidates for events where you want closures. You should be using easily trackable, class level state information instead of having the compiler create the closures for you. Closures in this case tend to lead to strange, difficult to track issues over time, and are not as maintainable.Woodrowwoodruff
I couldn't disagree with that more. Closures allow you to much more cleanly and succinctly pass in state. Creating little classes and setting state on them is gross and hacky and thus difficult to maintain.Edwyna
Regarding the debate on closures vs methods, I prefer closures by far. The main reason is that I don't want to add fields to a class unless necessary (read: two methods or more need to share variables). Using an additional type to hold the captured values is better, but since that's exactly what the compiler does for me with a closure, I'd rather use the closure.Macedonia
Seriously? Having local methods is just bad OO in a lot of cases. Using anonymous delegates is a far better approach. Just keep a reference for detaching the event handler - and wrap that in an aggregate IDisposable and you can easily clean up all of your event handlers.Trinitrophenol
"Having local methods is just bad OO", ummm...what?Cai
A
46

Give your instance of the anonymous delegate a name:

EventHandler<NewEventArg> handler = delegate(object sender, NewEventArgs e)
{
    //some code
};

Subject.NewEvent += handler;
Subject.NewEvent -= handler;
Aborn answered 28/8, 2009 at 16:40 Comment(8)
Why is this better than just making it a non-anonymous method? This is much, much less obvious.Woodrowwoodruff
@Aborn Don't you think this is very different from what I am asking?Unscrupulous
@PK: I think this is the closest you can get. You cannot unregister something that you cannot refer to.Aborn
@Reed Copsey: It's definitely not better and I'd recommend your answer. But if he wants anonymous delegates this is the closest he can get.Aborn
@Reed: Anonymous delegates have the added benefit of creating a closure, which you cannot do with a non-anonymous method. If the OP wants to be able to include an in-scope value that can't be passed into the eventargs then this is the best method.Tangerine
The code example most probably won't work, the event will most likely be never fired. Unsubscribing must take place inside the handler. I just posted a similar question here: #2147616Tauro
This might mean you'd need to create the delegate as a private member, if you're doing the unsubscribe in a different part of the class, so not much of a gain over using a methodLathan
Chris S: It's easier to tinker with the captured value if it's held in a member. If it's held in a closure (which is itself held as a member), tinkering is harder (impossible?). Moreover, in the case where more than a single value needs to be captured, the anonymous delegate solution is more concise.Macedonia
W
25

If you need to unregister an event, I recommend avoiding anonymous delegates for the event handler.

This is one case where assigning this to a local method is better - you can unsubscribe from the event cleanly.

Woodrowwoodruff answered 28/8, 2009 at 16:38 Comment(7)
I thought so. But, had a doubt. Thanks.Unscrupulous
I disagree - if you need to create a closure then you are going to have to use an anonymous method.Tangerine
@free-dom: There are always options to avoid making closures (worst case, you could do what the compiler does for you). Most of the time, event handlers where you plan to unsubscribe the event are not, IMO, good candidates for events where you want closures. You should be using easily trackable, class level state information instead of having the compiler create the closures for you. Closures in this case tend to lead to strange, difficult to track issues over time, and are not as maintainable.Woodrowwoodruff
I couldn't disagree with that more. Closures allow you to much more cleanly and succinctly pass in state. Creating little classes and setting state on them is gross and hacky and thus difficult to maintain.Edwyna
Regarding the debate on closures vs methods, I prefer closures by far. The main reason is that I don't want to add fields to a class unless necessary (read: two methods or more need to share variables). Using an additional type to hold the captured values is better, but since that's exactly what the compiler does for me with a closure, I'd rather use the closure.Macedonia
Seriously? Having local methods is just bad OO in a lot of cases. Using anonymous delegates is a far better approach. Just keep a reference for detaching the event handler - and wrap that in an aggregate IDisposable and you can easily clean up all of your event handlers.Trinitrophenol
"Having local methods is just bad OO", ummm...what?Cai
A
23

To remove the handler on first invocation:

//SubjectType Subject = ..... already defined if using (2)

EventHandler handler = null;
handler = delegate(object sender, EventArgs e)
{
    // (1)
    (sender as SubjectType).NewEvent -= handler;
    // or
    // (2) Subject.NewEvent -= handler;

    // do stuff here
};

Subject.NewEvent += handler;
Al answered 26/5, 2010 at 18:34 Comment(3)
Using this kind of code, Resharper complains about accessing a modified closure... is this approach reliable? I mean, are we sure that the 'foo' variable inside the body of the anonymous method, really references the anonymous method itself?Kavanaugh
I think I got the answer myself: Resharper is right, the captured variable ('handler' in the example above) is changed once the anonymous method get assigned to it. So, it does really change, but such mechanism ensures that the 'handler' variable stores a reference to the anonymous method itslef.Kavanaugh
I've had to do this so the handler could capture state (e.g. the calling method's parameters and locals) and state from the raiser of the event, all because the raiser didn't provide some needed information any other way prior to the calling method exited. It was tortuous, but it worked and didn't require creating an artificial class to handle the event.Duchess
M
6

You can create method for unregistering from all listeners of event. This not exactly what you whant, but sometimes it can be helpfull. For example (this really works =)) :

    class Program {
    static void Main(string[] args) {
        A someClass = new A();
        someClass.SomeEvent += delegate(object sender, EventArgs e) {
            throw new NotImplementedException();
        };

        someClass.ClearEventHandlers();
        someClass.FireEvent();

        Console.WriteLine("No error.");
    }

    public class A {
        public event EventHandler SomeEvent;

        public void ClearEventHandlers() {
            Delegate[] delegates = SomeEvent.GetInvocationList();
            foreach (Delegate delegate in delegates) {
                SomeEvent -= (EventHandler) delegate;
            }
        }

        public void FireEvent() {
            if (SomeEvent != null) {
                SomeEvent(null, null);
            }
        }
    }
}
Monotony answered 28/8, 2009 at 16:48 Comment(2)
Not sure why this was voted down -- of course, it's better to not have persistent anonymous methods in the first place, but if you are in a situation where you just need to clean them up, this works really well.Caravette
It's easier to just set someClass.SomeEvent = null instead of iterating thought InvocationList.Helminthic
T
2

You need a name for your anonymous function, and then, you can only do it as long as the name is in scope:

    var handler = new EventHandler(delegate(object o, EventArgs e)
    {
        //do something...
    };

    Subject.NewEvent += handler;

    // later on while handler is still in scope...

    Subject.NewEvent -= handler;
Tangerine answered 28/8, 2009 at 16:50 Comment(0)
H
0

Do you need to un-register it for a reason other than leakage?

Regarding the "Or just allow the memory to leak" bit, when Subject is cleaned up by the Garbage Collector, your anonymous delegate should be cleaned up as well, so there shouldn't be a leak.

Heredes answered 28/8, 2009 at 16:37 Comment(5)
Memory leak is one reason and other reason might be that I just want to stop listening to the eventUnscrupulous
Then you'd have to store it, as dtb's answer suggestsHeredes
Unfortunately, this can cause a leak. "this" will never get collected as long as "Subject" is still rooted, since the delegate behind Subject.NewEvent will hold a strong reference to "this" until Subject gets unrooted. The WeakEvent pattern exists for this exact reason.Woodrowwoodruff
@Reed: Ah, so if you use "this" in the anonymous delegate, then it creates a circular reference (object <-> delegate) that prevents the garbage collector from cleaning up? Is that what you mean?Heredes
no, the GC handles circular references well. However if Subject is never GC'ed (maybe it's e.g. the "MainFrame" that lives for the span of the application), then neither will the anonymous delegate, nor the implicit "this" in the delegate.Blink
M
0

There is another question (of mine) which goes into this in some (too much) detail: Weak event handler model for use with lambdas.

However, now that the Reactive Framework has come out, I'd seriously consider looking into that in this kind of situation.

Macropterous answered 15/9, 2010 at 11:57 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.