views:

105

answers:

3

Is it bad practice to write inline event handlers ?

For me, I prefer use it when I want to use a local variable in the event handler like the following:

I prefer this:

// This is just a sample
private void Foo()
{
    Timer timer = new Timer() { Interval = 1000 };
    int counter = 0; // counter has just this mission
    timer.Tick += (s, e) => myTextBox.Text = (counter++).ToString();
    timer.Start();
}

Instead of this:

int counter = 0; // No need for this out of Boo & the event handler

private void Boo()
{
    Timer timer = new Timer() { Interval = 1000 };

    timer.Tick += timer_Tick;
    timer.Start();
}

void timer_Tick(object sender, EventArgs e)
{
    myTextBox.Text = (counter++).ToString();
}
+8  A: 

It's absolutely fine - although there are two caveats:

  • If you're modifying a local variable from within a closure, you should make sure you understand what you're doing.
  • You won't be able to unsubscribe from the event

Typically I only inline really simple event handlers - for anything more involved, I use lambda expressions (or anonymous methods) to subscribe with a call to an method with a more appropriate method:

// We don't care about the arguments here; SaveDocument shouldn't need parameters
saveButton.Click += delegate { SaveDocument(); };
Jon Skeet
A: 

You put the two samples together. It is clear that the second option (which you don't prefer) is the most readable.

Code readability and maintainability are very important. Keep things simple, as easy as possible to understand. Lambda expressions are generally considered harder to understand by majority of people. Even if they are a second nature for you for others might not.

Liviu M.