CommandBars, Buttons and Commands: Cleanup is on the menu

One of the biggest pain point in the entire rubberduck code base, is the way commandbar menus and menu items are created and wired up - something needs to be done to straighten that up, and while I was refactoring the App class and got to the point where I needed to address the dependencies of RubberduckMenu, I had an idea: instead of creating menu buttons and wiring up event handlers, I could be creating commands and associating them with as many buttons as needed!

For example, I could have a RefactorRenameCommand that could be called from the Rubberduck/Refactorings menu, or from the code pane context menu, or from the form designer's context menu, or whatever.

So I created a Rubberduck.UI.Commands namespace, and wrote an abstract class to encapsulate the concept:

namespace Rubberduck.UI.Commands
{
    /// 
    /// Base class to derive all menu commands from.
    /// 
    public abstract class RubberduckCommandBase
    {
        private readonly IRubberduckMenuCommand _command;

        protected RubberduckCommandBase(IRubberduckMenuCommand command)
        {
            _command = command;
            _command.RequestExecute += command_RequestExecute;
        }

        private void command_RequestExecute(object sender, EventArgs e)
        {
            ExecuteAction();
        }

        protected IRubberduckMenuCommand Command { get { return _command; } }

        /// 
        /// A method that uses the  helper to wire up as many UI controls as needed.
        /// 
        public abstract void Initialize();

        /// 
        /// The method that is executed when either wired-up UI control is clicked.
        /// 
        public abstract void ExecuteAction();

        public void Release()
        {
            _command.Release();
        }
    }
}

The IRubberduckMenuCommand interface defines everything I need to be able to associate "buttons" with a command, to destroy the created COM objects on demand (changing the app's language from

This looks good! I've got a couple of comments to make in a vague, hand wavy way so I'll start off with a comment about the code as written:

public SomeMethod(int beforeIndex = -1)
{
    if (beforeIndex == -1) 
    {

No! Bad Mug. No presents for you this Christmas. Use a Nullable and be more idiomatic:

public SomeMethod(int? beforeIndex = null)
{
    if (beforeIndex.HasValue) 
    {

There's also some magic numbery going on here:

_vbe.CommandBars[1].Controls.OfType()
            .SingleOrDefault(control => control.Caption == RubberduckUI.RubberduckMenu);

You could do something like:

_vbe.CommandBars.SelectMany(commandBar => commandBar.Controls)
                .OfType()
                .SingleOrDefault(control => control.Caption == RubberduckUI.RubberduckMenu);

Or name your constant. E.g. const int RubberDuckMenuBarIndex = 1.

Anyway, the code looks good - nice names, nice spacing, great formatting in general. I'm assuming IPictureDisp isn't one of your own interfaces?

So, on to some dangerous hand waving.

I feel like you need to make the seperation between displying the buttons and wiring up the command a bit more obvious. I think the way you can achieve that is with the mediator pattern.

You basically have a central hub that sends messages to the right things to do the right thing. All of your buttons could then do the same thing - send a message to the mediator.

var result = Mediator.Send(new AboutClickedCommand());

Then you have some sort of iterfaces like:

public interface ICommand { }

public interface ICommandHandler where T : ICommand
{
    TResult Handle(T command);
}

public class Mediator
{
    public TResult Send (ICommand command)
    {
        var handler = LookUpHandlerForCommand(command);
        if (handler != null)
        {
            return handler.Handle(command);
        }
        throw new HandlerMissingException();
    }
}

So your mediator simply looks up a handler and passes the message on and returns the result back. I haven't used it, but Jimmy Bogard has a library called Mediatr which is supposedly very good.

Your buttons would simply encapsulate the rules about displaying themselves, your ICommandHandlers would have all of the stuff about what they should do. You also allow the handler to report back to the thing that called it - quite a useful thing to have. You can also add properties to your command objects to pass in additional information/context.

Hopefully that makes sense and I haven't messed up the generics in my sample code...

StackExchange Code Review Q#98793, answer score: 7