Event unsubscription via anonymous delegate [duplicate]

Question

I am using Resharper 5.1 code analysis many a times i get a comment from resharper as

"Event unsubscription via anonymous delegate"

#Part of Code  

if (((bool)e.NewValue))
{
    listView.PreviewTextInput += (o,args) =>
        listView_PreviewTextInput(o,args,listView);
}
else
{
    listView.PreviewTextInput -= (o, args) => 
        listView_PreviewTextInput(o, args, listView);
}

How could i correct or optimze this thing

Hardi

Answer 1

You can extract the lamdba to a variable:

EventHandler func = (sender, e) =>
    listView_PreviewTextInput(sender, e, listView);

if (((bool)e.NewValue))
{
    listView.PreviewTextInput += func;
}
else
{
    listView.PreviewTextInput -= func;
}

Pueblo

Thanks but the EventHandler should be specific right ?? Cuz its giving me an error ... System.EvenTArgs is not assignable to TextCompositonEventArgs – Hardi

In that case listView.PreviewTextInput is not an EventHandler but probably a EventHandler<TextCompositonEventArgs>, but there is no way for me to know, since you didn't show that in your question. – Pueblo

Yup.. my bad.. Thanks ne ways ... :) – Hardi

Answer 2

Warning! Accepted answer from Steven is wrong, all it does is just masking a problem that resharper is warning about.

Every time given code is executed

 EventHandler func = (sender, e) =>
     listView_PreviewTextInput(sender, e, listView);

you'll get a fresh (since you may capture different listView) instance of anonymous delegate saved to func, an instance that's not subscribed to any events yet, so in turn this code

listView.PreviewTextInput -= func;

will effectively do nothing, since you cant unsubscribe from an event you're not subscribed to. This will lead to mind-boggling bugs like event handlers 'called twice', memory leaks etc.

Actually, Jon Skeet says it may work in some cases:

The C# specification explicitly states (IIRC) that if you have two anonymous functions (anonymous methods or lambda expressions) it may or may not create equal delegates from that code.

e.g. when compiler doesn't generate new instance every time, you'll see nice behavior.

But that's not reliable and certainly wouldn't work in case described in starter question with captured variable listView.

So my suggestion is:

Use anonymous functions as event handlers ONLY if you will never have to unsubscribe.

Borlase

so what would be the right way to do it? – Magnific

@Magnific turn anonymous delegate to a 'normal' method or local function – Borlase

NOTE: Can't "turn it into a normal method" in the original case, because of the extra parameter listView. Maybe local function, but I'm not certain that local function is any more reliable than the lambda, because of this: "Note that when a local function captures variables in the enclosing scope, the local function is implemented as a delegate type." in Local functions. To me that might mean that its not the same instance. IF the extra parameters don't change, can store in field. – Neisse