HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

CommandBars, Buttons and Commands: Take 2

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
commandstakebuttonsandcommandbars

Problem

Following-up on CommandBars, Buttons and Commands: Cleanup is on the menu, I decided to try a more ambitious approach, as suggested in Nikita's answer. It works, but there are a number of "gotchas" that I'm not sure how to clean up.

So I have an ICommand interface, and its generic counterpart:

public interface ICommand
{
    void Execute();
}

public interface ICommand : ICommand
{
    void Execute(T parameter);
}


Note that there's no CanExecute method, because I haven't yet figured out where the VBE API or the Office CommandBar API is giving me a decent hook to execute it.

I haven't implemented any parameterized commands yet, but I'm thinking the parameterless overload will call the parameterized one after figuring out the parameter to use... but I'll stick to the "About" menu command for this post.

public class AboutCommand : ICommand
{
    public void Execute()
    {
        using (var window = new _AboutWindow())
        {
            window.ShowDialog();
        }
    }
}

public class AboutCommandMenuItem : CommandMenuItemBase
{
    public AboutCommandMenuItem(ICommand command)
        : base(command)
    {
    }

    public override string Key { get { return "RubberduckMenu_About"; } }
}


The CommandMenuItemBase is an abstract class that immensely facilitates implementing menu commands - all that's left to do is to specify the resource key that contains the caption.

```
public abstract class CommandMenuItemBase : ICommandMenuItem
{
private readonly ICommand _command;

protected CommandMenuItemBase(ICommand command)
{
_command = command;
}

public abstract string Key { get; }

public ICommand Command { get { return _command; } }
public Func Caption { get { return () => RubberduckUI.ResourceManager.GetString(Key, RubberduckUI.Culture); } }
public bool IsParent { get { return false; } }
public virtual Image Image { get { return null; } }
public virtual Image Mask { get { return null; } }

Solution

I think the code is definitely much better! I wouldn't worry too much about registering the ICommands in your IoC. After all, you're not going to have hundreds of menu items yet.

Anyway, two very minor comments:

= beginGroup ?? false;


is better as

= beginGroup.GetValueOrDefault();


And this:

private AxHostConverter() : base("") { }


Would be better as:

private AxHostConverter() : base(string.Empty) { }


Edit

With the caveat that I don't know ninject and that this isn't very performant here's a reflection based approach for registration as a complete console app:

internal class Program
{
    static void Main(string[] args)
    {
        IKernel kernel = new StandardKernel();

        var query = from command in Assembly.GetExecutingAssembly().GetTypes()
                    where command.GetInterfaces().Contains(typeof(ICommand)) && command.IsClass
                    select
                        new Tuple(
                        command,
                        FindCorrespondingType(command));

        foreach (var item in query)
        {
            kernel.Bind().To(item.Item1).WhenInjectedExactlyInto(item.Item2);
        }

        kernel.Get();
        kernel.Get();
        Console.ReadKey();
    }

    private static Type FindCorrespondingType(Type command)
    {
        return Assembly.GetExecutingAssembly().GetTypes().FirstOrDefault(t => t.Name == $"{command.Name}MenuItem");
    }
}

public interface ICommand
{ }

public class AboutCommand : ICommand
{
}

public class OptionsCommand : ICommand
{

}

public class AboutCommandMenuItem
{
    public AboutCommandMenuItem(ICommand comman)
    {
        Console.WriteLine(comman.GetType());
    } 
}

public class OptionsCommandMenuItem
{
    public OptionsCommandMenuItem(ICommand comman)
    {
        Console.WriteLine(comman.GetType());
    }
}

Code Snippets

= beginGroup ?? false;
= beginGroup.GetValueOrDefault();
private AxHostConverter() : base("") { }
private AxHostConverter() : base(string.Empty) { }
internal class Program
{
    static void Main(string[] args)
    {
        IKernel kernel = new StandardKernel();

        var query = from command in Assembly.GetExecutingAssembly().GetTypes()
                    where command.GetInterfaces().Contains(typeof(ICommand)) && command.IsClass
                    select
                        new Tuple<Type, Type>(
                        command,
                        FindCorrespondingType(command));

        foreach (var item in query)
        {
            kernel.Bind<ICommand>().To(item.Item1).WhenInjectedExactlyInto(item.Item2);
        }

        kernel.Get<AboutCommandMenuItem>();
        kernel.Get<OptionsCommandMenuItem>();
        Console.ReadKey();
    }

    private static Type FindCorrespondingType(Type command)
    {
        return Assembly.GetExecutingAssembly().GetTypes().FirstOrDefault(t => t.Name == $"{command.Name}MenuItem");
    }
}

public interface ICommand
{ }

public class AboutCommand : ICommand
{
}

public class OptionsCommand : ICommand
{

}

public class AboutCommandMenuItem
{
    public AboutCommandMenuItem(ICommand comman)
    {
        Console.WriteLine(comman.GetType());
    } 
}

public class OptionsCommandMenuItem
{
    public OptionsCommandMenuItem(ICommand comman)
    {
        Console.WriteLine(comman.GetType());
    }
}

Context

StackExchange Code Review Q#98979, answer score: 4

Revisions (0)

No revisions yet.