Best way to handle MenuItem Click events?

Question

I'm wondering what is the best way to handle 30 or so Click events for MenuItems?

My first thought was obviously to just create an event listener for each MenuItem like so:

XAML:

<Menu Name="MainMenu" IsMainMenu="True" Height="25">
        <MenuItem Header="_File" >
            <MenuItem Name="New" Header="_New" Click="MenuItem_NewClick" />
            <MenuItem Name="Open" Header="_Open" Click="MenuItem_OpenClick" />
            <MenuItem Name="Save" Header="_Save" Click="MenuItem_SaveClick" />
        </MenuItem>
</Menu>

C# 1:

private void MenuItem_NewClick(object sender, RoutedEventArgs e)
{           
    //Do work...
}
private void MenuItem_OpenClick(object sender, RoutedEventArgs e)
{
    //Do work...
}
private void MenuItem_SaveClick(object sender, RoutedEventArgs e)
{
    //Do work...
}

But this seems messy, especially for MenuItems that dont need much code like Copy or Paste.

I could instead use one event listener and use a if/case to check the MenuItem and eliminate all the extra event listeners like so:

C# 2:

private void MenuItem_FileClick(object sender, RoutedEventArgs e)
    {
        MenuItem item = e.OriginalSource as MenuItem;
        switch (item.Name)
        {
            case "New":
                MessageBox.Show("New File Created.");
                break;
            case "Open":
                MessageBox.Show("File Opened Created.");
                break;
            case "Save":
                MessageBox.Show("File Saved.");
                break;
        }
    }

Which seems much cleaner, but at the same time also messy if the MenuItems need to do more work than just open another form or clear a few controls. If complex logic is need I could call a function but I think that would lead to spaghetti code?

What is the most elegant way to handle this?

Thanks, Tanner

Answer 1

I often do this kind of thing to set up actions where I'm trying to separate the UI from the actions being performed.

        var actions = new Dictionary<string, Func<MenuItem, RoutedEventHandler>>()
        {
            { "New", mi => (s, e) => { MessageBox.Show("New File Created."); }},
            { "Open", mi => (s, e) => { MessageBox.Show("File Opened."); }},
            { "Save", mi => (s, e) => { MessageBox.Show("File Saved."); }},
        };

        foreach (MenuItem mi in FileMenu.Items)
        {
            if (actions.ContainsKey(mi.Name))
            {
                mi.Click += actions[mi.Name](mi);
            }
        }

The nice thing about this is that you explicitly have a strongly-type reference to the mnu item. You could do this, for example, when defining each item:

            {
                "New",
                mi =>
                    (s, e) =>
                    {
                        MessageBox.Show("New File Created.");
                        MessageBox.Show(
                            String.Format("You clicked the {0} menu.", mi.Name));
                    }
            },

You can see that that mi variable is passed in to the lambda expression.

You do need to do the opposite to detach your event handlers when you exit the form, but it's no harder than the attach code. If you're clever you can write code that prepares the detach when you do the attach to make it super easy to detach later.