Constructor-injecting a truckload of commands

Question

There's a problem in Rubberduck that we haven't neatly solved yet, and as we're exploring alternative approaches I figured I'd ask the CR community and see what our C# experts think.

Here's the slightly simplified (irrelevant constructor parameters were removed) constructor for the CodeExplorerViewModel (actual file on GitHub), which is used by the WPF/XAML control for the Code Explorer feature.

The ViewModel needs to expose quite a large number of commands, and since each command has its dependencies, and these dependencies have their own dependencies, we use DI/IoC to constructor-inject the commands into the ViewModel - the DI registration essentially injects every single command instance that exists in Rubberduck all Code Explorer commands¹, and then since the XAML command bindings require ICommand properties, we assign these properties by fetching the command of the appropriate type from the List<CommandBase> we're given:

public CodeExplorerViewModel(List<CommandBase> commands)
{
    var reparseCommand = commands.OfType<ReparseCommand>().SingleOrDefault();

    RefreshCommand = new DelegateCommand(LogManager.GetCurrentClassLogger(), 
        reparseCommand == null ? (Action<object>)(o => { }) :
        o => reparseCommand.Execute(o),
        o => !IsBusy && reparseCommand != null && reparseCommand.CanExecute(o));

    OpenCommand = commands.OfType<UI.CodeExplorer.Commands.OpenCommand>().SingleOrDefault();
    OpenDesignerCommand = commands.OfType<OpenDesignerCommand>().SingleOrDefault();

    AddTestModuleCommand = commands.OfType<UI.CodeExplorer.Commands.AddTestModuleCommand>().SingleOrDefault();
    AddStdModuleCommand = commands.OfType<AddStdModuleCommand>().SingleOrDefault();
    AddClassModuleCommand = commands.OfType<AddClassModuleCommand>().SingleOrDefault();
    AddUserFormCommand = commands.OfType<AddUserFormCommand>().SingleOrDefault();

    OpenProjectPropertiesCommand = commands.OfType<OpenProjectPropertiesCommand>().SingleOrDefault();
    RenameCommand = commands.OfType<RenameCommand>().SingleOrDefault();
    IndenterCommand = commands.OfType<IndentCommand>().SingleOrDefault();

    FindAllReferencesCommand = commands.OfType<UI.CodeExplorer.Commands.FindAllReferencesCommand>().SingleOrDefault();
    FindAllImplementationsCommand = commands.OfType<UI.CodeExplorer.Commands.FindAllImplementationsCommand>().SingleOrDefault();

    CollapseAllSubnodesCommand = new DelegateCommand(LogManager.GetCurrentClassLogger(), ExecuteCollapseNodes);
    ExpandAllSubnodesCommand = new DelegateCommand(LogManager.GetCurrentClassLogger(), ExecuteExpandNodes);

    ImportCommand = commands.OfType<ImportCommand>().SingleOrDefault();
    ExportCommand = commands.OfType<ExportCommand>().SingleOrDefault();
    ExportAllCommand = commands.OfType<Rubberduck.UI.Command.ExportAllCommand>().SingleOrDefault();

    _externalRemoveCommand = commands.OfType<RemoveCommand>().SingleOrDefault();
    if (_externalRemoveCommand != null)
    {
        RemoveCommand = new DelegateCommand(LogManager.GetCurrentClassLogger(), ExecuteRemoveComand, _externalRemoveCommand.CanExecute);
    }

    PrintCommand = commands.OfType<PrintCommand>().SingleOrDefault();

    CommitCommand = commands.OfType<CommitCommand>().SingleOrDefault();
    UndoCommand = commands.OfType<UndoCommand>().SingleOrDefault();

    CopyResultsCommand = commands.OfType<CopyResultsCommand>().SingleOrDefault();

    SetNameSortCommand = new DelegateCommand(LogManager.GetCurrentClassLogger(), param =>
    {
        if ((bool)param == true)
        {
            SortByName = (bool)param;
            SortByCodeOrder = !(bool)param;
        }
    }, param =>
    {
        return SortByName ? false : true;
    });

    SetCodeOrderSortCommand = new DelegateCommand(LogManager.GetCurrentClassLogger(), param =>
    {
        if ((bool)param == true)
        {
            SortByCodeOrder = (bool)param;
            SortByName = !(bool)param;
        };
    }, param => 
    {
        return SortByCodeOrder ? false : true;
    });
}

Some of these commands are used in command bindings for the toolbar, some are used in the context menu, others in link buttons in the bottom panel, and some are reused/redundant, for UX/convenience:

Having as many constructor parameters as we need to expose commands would be unacceptable.

Surely there's a better way to do this?

Note: We're currently using ninject, but the IoC is being ported to castle-windsor.

---

_{¹ [CodeExplorerCommand] attributes decorate the Code Explorer command classes; the IoC configuration is setup so that only command classes decorated with that attribute get injected into the ViewModel.}

Answer 1

After rethinking the problem, I got another idea. Because it is fundamentally different, I'll post it in a separate answer.

The commands are injected to the view model, exposed by properties and bound to the view, right? Why not just setting the commands directly in the view?

The following markup extension makes it possible:

public class CommandResolver : MarkupExtension
{
    private readonly Type myCommandType;
    public CommandResolver(Type commandType)
    {
        if (commandType == null)
            throw new ArgumentNullException(nameof(commandType));
        if (!typeof(ICommand).IsAssignableFrom(commandType))
            throw new ArgumentException($"Type '{commandType}' has to implement ICommand.");

        myCommandType = commandType;
    }

    public override object ProvideValue(IServiceProvider serviceProvider)
    {
        var isDesignMode = DesignerProperties.GetIsInDesignMode(new DependencyObject());

        if (!isDesignMode)
        {
            Debug.Assert(Resolve != null, "Resolve must not be null. Please initialize resolving delegate during application startup.");
        }

        return Resolve(myCommandType) ?? DependencyProperty.UnsetValue;
    }

    public static Func<Type, object> Resolve { private get; set; }
}

It must be initialized during applications startup:

var kernel = new StandardKernel();
// ... initialize Ninject kernel (can be used with any other DI container)

CommandResolver.Resolve = type => kernel.Get(type);

The resolver can be used in XAML to probide property types directly:

<Button Command="{resolverns:CommandResolver commandsns:OpenCommand}" />

Answer 2

Ninject has a very nice extension called Ninject.Extensions.Factory. It allows you to create factories by specifying an interface. In your case, you could create a factory that creates the commands:

public interface ICommandFactory
{
    OpenCommand CreateOpenCommand();
    OpenDesignerCommand CreateOpenDesignerCommand();
    // ...
}

Note that dependencies, defined in command's contructors, are automatically injected by the factory extension (Internally, it holds an instance of the kernel).

The factory can be used to create the commands in constructor (or even on the fly if the command is required):

public CodeExplorerViewModel(ICommandFactory commandFactory)
{
    OpenCommand = commandFactory.CreateOpenCommand();
    OpenDesignerCommand = commandFactory.CreateOpenDesignerCommand();
    // ....
}

For me it feels better to use a factory for creating commands compared to registering all commands to the DI container. However, it is more effort to create and maintain the factory interface.

[Edit]

Alternatively, it is possible to create a generic factory for commands. That doesn't require to extend / maintain the interface:

public interface ICommandFactory
{
    TCommand CreateCommand<TCommand>() where TCommand : CommandBase;
}

// ...

public CodeExplorerViewModel(ICommandFactory commandFactory)
{
    OpenCommand = commandFactory.CreateCommand<OpenCommand>();
    OpenDesignerCommand = commandFactory.CreateCommand<OpenDesignerCommand>();
    // ....
}

This solution is simpler to use but not far away from passing the kernel directly. ;).

Answer 3

I would typically create a manager for this. Make a class that contains all the properties for each command. Not a factory because we will still allow the DI container to create the commands we just making a nicer way to access them.

Now I don't want to keep updating the manager each time a new command is created. Which is why the t4 templates were created. We can make a template that reads the command directory and creates a property for each command and assigns that value in the constructor.

I created a simple t4 template called CommandManager.tt

<#@ template debug="false" hostspecific="true" language="C#" #>
<#@ assembly name="System.Core" #>
<#@ import namespace="System.IO" #>
<#@ import namespace="System.Linq" #>
<#@ import namespace="System.Text" #>
<#@ import namespace="System.Collections.Generic" #>
<#@ output extension=".cs" #>

<#
   // whatever namespace you want.  If you want to read namespace hints there are stack overflow examples this is just a simple example
   var namespaceName = "Rubberduck.UI.CodeExplorer.Commands";
   var t4 = Host.TemplateFile;
   // Realtive path to commands from t4 template
   var path = Path.Combine(Path.GetDirectoryName(t4), @"..\..\..\Commands");
   var commands = Directory.GetFiles(path, "*Command.cs").Select(Path.GetFileNameWithoutExtension).ToArray();
#>

using System.Collections.Generic;
using System.Linq;
using System.Windows.Input;

namespace <#= namespaceName #>
{
    public class <#= Path.GetFileNameWithoutExtension(t4) #>
    {
        public <#= Path.GetFileNameWithoutExtension(t4) #>(IList<ICommand> commands)
        {
<#
   foreach (var command in commands)
   {
#>
           <#=command #> = commands.OfType<<#=namespaceName #>.<#=command #>>().SingleOrDefault();
<#
   }
#>
        }

<#
   foreach (var command in commands)
   {
#>
        public ICommand <#=command #> {get; }
<#
   }
#>
    }
}

I would register this class as a singleton and I made the properties read-only. This will create a class something like this

using System.Collections.Generic;
using System.Linq;
using System.Windows.Input;

namespace Rubberduck.UI.CodeExplorer.Commands
{
    public class CommandManager
    {
        public CommandManager(IList<ICommand> commands)
        {
           AddClassModuleCommand = commands.OfType<Rubberduck.UI.CodeExplorer.Commands.AddClassModuleCommand>().SingleOrDefault();
           AddStdModuleCommand = commands.OfType<Rubberduck.UI.CodeExplorer.Commands.AddStdModuleCommand>().SingleOrDefault();
           ExportCommand = commands.OfType<Rubberduck.UI.CodeExplorer.Commands.ExportCommand>().SingleOrDefault();
           PrintCommand = commands.OfType<Rubberduck.UI.CodeExplorer.Commands.PrintCommand>().SingleOrDefault();
        }

        public ICommand AddClassModuleCommand {get; }
        public ICommand AddStdModuleCommand {get; }
        public ICommand ExportCommand {get; }
        public ICommand PrintCommand {get; }
    }
}

Notes about this. This reads the file name and assumes the class is the same as the file. If not the linq shouldn't fail the command will just be null. Up to you if you want it to throw at runtime so you know you have an issue or not.

Also I made the class partial so if there is anything special that you need added that doesn't have to be in the t4 template you can create another partial class and add it there.

Now I would inject the CommandManger class into your ViewModel and set it's property from properties from the CommandManager. This class now is reusable in all your viewmodels and each one doesn't have to iterate over the list of commands to find the one it needs.

Answer 4

After reading all discussions/answers here I come to this solution. To be clear I just adopted @JanDotNet 's solution for ViewModel. So let's start.

First, we don't want to have huge amount of useless command-properties declared. So we need to archive typesafe access from View or ViewModel (As I see there is similar case in Rubberduck) without declaring properties.In second, we won't having external dependencies for our view. Third, have you seen this mem ?

I want to say that this solution isn't intended to be the best. I just want to share my solution. ( I had similar problem. )

CommandBindings
In this the data structure we will store our commands.

public class CommandBindings
{
    public CommandBindings(IEnumerable<ICommand> commands)
    {
        _underlyingCommands = commands ?? throw new ArgumentNullException(nameof(commands));
    }

    public ICommand OfType(Type type)
    {
        return _underlyingCommands.FirstOrDefault(x => x.GetType() == type);
    }

    private IEnumerable<ICommand> _underlyingCommands;
}

CommandBindingsProvider
This service will provide our CommandBindings, by retrieving commands with appropriate attributes. ( or however it is necessary )

public class CommandBindingsProvider : ICommandBindingsProvider
{
    public static Func<Type, object> Resolve { private get; set; }

    public CommandBindings GetBindings()
    {
        if (Resolve == null)
            throw new InvalidOperationException("CommandBindngs Resolve must be set in application startup.");

        var commands = new List<ICommand>();

        var commandTypes = GetCommandTypesByAttribute();
        foreach (var commandType in commandTypes)
        {
            var command = Resolve(commandType) as ICommand;
            Debug.Assert(command != null, $"Command {commandType.Name} isn't registered in container.");
            if (command != null)
            {
                commands.Add(command);
            }
        }

        var bindings = new CommandBindings(commands);
        return bindings;
    }

    private static IEnumerable<Type> GetCommandTypesByAttribute()
    {
        ...
    }
}

CommandBindingsProvider.Resolve must be initialized during application startup:

var kernel = new StandardKernel();
// ... initialize Ninject kernel (can be used with any other DI container)  
CommandBindingsProvider.Resolve = type => kernel.Get(type);

IVMWithCommandBindings
ViewModel that has commands with CommandBindings will implement this interface.

public interface IVMWithCommandBindings
{
    CommandBindings CommandBindings { get; }
}

Now our ViewModel:

public class ExplorerVM : IVMWithCommandBindings
{
    public ExplorerVM(ICommandBindingsProvider commandBindingsProvider)
    {
        CommandBindings = commandBindingsProvider.GetBindings();
    }

    public CommandBindings CommandBindings { get; }
}

We have our ViewModel with CommandBindings. Now we must define a way of using this CommandBindings in XAML. Markup extension comes here:

CommandResolver

public class CommandResolver : MarkupExtension
{
    public CommandResolver(Type commandType)
    {
        if (commandType == null)
            throw new ArgumentNullException(nameof(commandType));
        if (!typeof(ICommand).IsAssignableFrom(commandType))
            throw new ArgumentException($"Type '{commandType}' has to implement ICommand.");

        myCommandType = commandType;
    }

    public override object ProvideValue(IServiceProvider serviceProvider)
    {
        var tp = (IProvideValueTarget)serviceProvider.GetService(typeof(IProvideValueTarget));
        if (tp == null) return DependencyProperty.UnsetValue;

        var target = tp.TargetObject as FrameworkElement;
        if (target == null) return DependencyProperty.UnsetValue;

        var vmWithCommandBindings = target.DataContext as IVMWithCommandBindings;
        if (vmWithCommandBindings == null)
            throw new InvalidOperationException("CommandResolver is designed to be use with IVMWithCommandBindings data contexts.");

        return vmWithCommandBindings.CommandBindings.OfType(myCommandType) ?? DependencyProperty.UnsetValue;
    }

    private readonly Type myCommandType;
}

Here we retrieve element's DataContext in which markup extension is used. Then we check that DataContext object implements IVMWithCommandBindings interface, after that we resolve appropriate command with passed type.

Finally:

Usage in XAML:

<Button Command="{resolvers:CommandResolver commands:TestCommand}"/>

Usage in ViewModel:

CommandBindings.OfType(typeof(TestCommand)).Execute(null);

Looks ugly, but if we write Extension for IVMWithCommandBindings interface like this:

public static class VMWithCommandBindingsExtensions
{
    public static void ExecuteCommand(this IVMWithCommandBindings vm, Type type, object param = null)
    {
        // skipping null checks for brevity sake 
        var command = vm.CommandBindings.OfType(type);
        if (command.CanExecute(param))
        {
            command.Execute(param);
        }
    }
}

we will have Usage in ViewModel like:

this.ExecuteCommand(typeof(TestCommand));