2
\$\begingroup\$

I have a class with approximately 140 menu buttons in a nested ribbon-style menu over a canvas type area within a WinForms application.

As a result of this, a large swathe of my codebehind file consists of 140 event handlers for click events.

Many of them however, are remarkable similar, for example:

    private void btnFinancialStatusReport(object sender, EventArgs e)
    {
        SwitchCurrentControl(new FinancialStatusReport());
    }

    private void btnMonthlySalesReport(object sender, EventArgs e)
    {
        SwitchCurrentControl(new MonthlySalesReport());
    }

    private void btnDailySalesReport(object sender, EventArgs e)
    {
        SwitchCurrentControl(new DailySalesReport());
    }

SwitchCurrentControl is a private method with the responsibility of essentially taking the newly constructed UserControl and docking it correctly into the active canvas. This is irrelevant to the question but I thought somebody might ask...

The Question

For this many buttons, (140+) a good 50% of them are practically the same, the only difference being in which Control they create.

Is this acceptable, or is there a way I can refactor this to reduce the amount of handlers?

Note

Before anybody suggests I switch to WPF btw, that is not an option...unless that same person wants to come and redo all 140 associated controls for me ;-)

\$\endgroup\$
4
\$\begingroup\$

You can create a Dictionary, where key is a MenuItem, a value us function/delegate that has to be called on click. All 140+ menu items subscribe to the same event hadler and in that event handler write something like

myMenuItemsDictionary[((MenuItem)sender)]()

this is good, in case when you have no parameters to pass to a delegate, in case when you need to pass them, you can go farther:

define a class

public class MenuItemHandler
{
    delegate MethoddToInvokeOnClick()
    List<object> parametersToPassToDelegate = ..
}

and the instances of this class have in a Value of the dictionary

\$\endgroup\$
  • 1
    \$\begingroup\$ How would you use the Class MenuItemHandler? Would you parse the dictionary entry and try to extract the MethodToInvoke from the value-string in the dictionary? Could you give a brief example? \$\endgroup\$ – surfmuggle May 16 '13 at 10:16
3
\$\begingroup\$

Something like this?

btnFinancialStatusReport.Click += delegate(object sender, EventArgs args) { btnGenericClick<FinancialStatusReport> (sender, args); }; 
btnMonthlySalesReport.Click += delegate(object sender, EventArgs args) { btnGenericClick<btnMonthlySalesReport> (sender, args); }; 

private void btnGenericClick<T>(object sender, EventArgs e) where T: new()
{
    SwitchCurrentControl(new T());
}
\$\endgroup\$
  • \$\begingroup\$ Good idea; however, you need to a the constraint where T : new(). \$\endgroup\$ – Olivier Jacot-Descombes Apr 26 '12 at 15:39
  • \$\begingroup\$ It's also impossible to detach an anonymous delegate from an event unless you keep a named reference to it elsewhere; this may or may not be a concern to the OP. \$\endgroup\$ – KeithS Apr 26 '12 at 17:03
  • \$\begingroup\$ Can't say I'm a fan of creating a generic method where none of the inputs or outputs use the generic parameter(s). That makes the use confusing. \$\endgroup\$ – Jeff Mercado Apr 28 '12 at 15:54
2
\$\begingroup\$

You could use a "curried" anonymous delegate...

public static Func<object, EventArgs> CurryWith<T>(this Func<Func<T>, object, EventArgs> input, Func<T> func)
{
    return (s,e) => input(func(), s, e);
}

...

Func<Func<Control>, object, EventArgs> switchHandler = (f, s, e)=> SwitchCurrentControl(f());

...

btnFinancialStatusReport.Click += switchHandler.CurryWith(()=>new FinancialStatusReport());
btnMonthlySalesReport.Click += switchHandler.CurryWith(()=>new MonthlySalesReport());
btnDailySalesReport.Click += switchHandler.CurryWith(()=>new DailySalesReport());

Personally if I were doing this myself I would set up a factory that could produce various reports based on input identifying the type of report to produce. Tigran's idea of a Dictionary of delegates is an excellent one.

\$\endgroup\$
0
\$\begingroup\$

The handlers are very simple and can hardly be simplified; However, by assigning the handlers programmatically as lambda expressions, you can turn them into one-liners

btnFinancialStatusReport.Click += (sender, e) =>  SwitchTo(new FinancialStatusReport());
btnMonthlySalesReport.Click += (sender, e) =>  SwitchTo(new MonthlySalesReport());
btnDailySalesReport.Click += (sender, e) => SwitchTo(new DailySalesReport());     
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.