Lambda closure or class level variable?

Question

Just a general question about what the best practice is:

        public void Foo()
        {
            int x = 5;
            myControl.Click += (o, e) =>
            {
                x = 6;
            };

        }

Notice, I'm using the x variable inside my lambda event handler.

OR:

     public class Bar
    {
        private int x = 5;
        public void Foo()
        {
            Control myControl = new Control();
            myControl.Click += new EventHandler(myControl_Click);
        }

        private void myControl_Click(object sender, EventArgs e)
        {
            x = 6;
        }
    }

Here, x is a private member of the class, and therefore I have access to it in my event handler.

Now let's say I don't need x anywhere else in the code (for whatever reason), which method is the better way to go?

Answer 1

If you don't need x anywhere else in the code, your handler is a no-op - so surely that's a nonsense situation.

Once you do need x, you need to decide whether it should be scoped to the Bar instance or the delegate instance (or possibly some collection of delegates), and that will dictate what you do.

Answer 2

It depends on your need. In the first example, the side-effects of your event handler are limited to method scope, and in the second, the side-effects are instance scoped. I think that using a closure in terms of your first example serves no purpose since X isn't used anywhere, so it's hard to determine based on your examples.

That being said, it is generally best to treat event handlers (that you create in code) as you would variables. Scope them as narrowly as you can, and refactor them to broader scope as needed.

A better example that highlights when you should use a closure is as follows:

public void Subscribe(Action<string> messageCallBack)
{
    myButton.Click += () => messageCallBack("Button was clicked.");
}

This allows multiple subscribers, and is much simpler than the alternative:

private readonly List<Action<string>> callBacks;
public MyClass()
{
    callBacks = new List<Action<string>>();
    myButton.Click += myButton_Click;
}

private myButton_Click(object sender, EventArgs e)
{
    foreach (Action<string> callBack in callBacks)
    {
        callBack("Button was clicked");
    }
}

public void Subscribe(Action<string> messageCallBack)
{
    callBacks.Add(messageCallBack);
}