3
\$\begingroup\$

For example I have this property in my ViewModel.

public IStorageFolder CurrentFolder
{
    get
    {
        return _currentFolder;
    }
    set
    {
        Set(ref _currentFolder, value);
        CommandBindings[nameof(NavigateCommand)].Execute(Model);
    }
}

private IStorageFolder _currentFolder = null;

CommandBindings.cs

public class CommandBindings
{
    ...

    public ICommand this[string name]
    {
        get
        {
            return GetCommandByName(name);
        }
    }

    public ICommand GetCommandByName(string name)
    {
       ...
    }
}

Navigate.cs

public class NavigateCommand : ExplorerCommand
{
    public override bool CanExecute(IExplorerModel model)
    {
        return true;
    }

    public override async Task ExecuteAsync(IExplorerModel model)
    {
        ....
    }
}

CommandBindingsProvider.cs

public class CommandBindingsProvider : ICommandBindingsProvider
{
    public CommandBindings GetBindings()
    {
        var commandInfos = new List<CommandInfo>();
        var serviceLocator = ServiceLocator.Current;
        var explorerCommandTypes = GetExplorerCommandTypes();

        foreach (var commandType in explorerCommandTypes)
        {
            var command = serviceLocator.GetInstance(commandType) as ICommand;
            if (command == null) throw new Exception($"No instance registered for {commandType.FullName} command.");
            var commandName = commandType.Name;
            var commandInfo = new CommandInfo(commandName, command);
            commandInfos.Add(commandInfo);
        }

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

    private static IEnumerable<Type> GetExplorerCommandTypes()
    {
        var assembly = typeof(ExplorerCommand).GetTypeInfo().Assembly;
        foreach (Type type in assembly.GetTypes())
        {
            if (type.GetTypeInfo().GetCustomAttributes(typeof(ExplorerCommandAttribute), true).Count() > 0)
            {
                yield return type;
            }
        }
    }
}

Commands are registered in CommandBindings by nameof class. I execute them by using this syntax in code:

CommandBindings[nameof(NavigateCommand)].Execute(Model);

and by this syntax in XAML :

<ic:InvokeCommandAction Command="{Binding CommandBindings[NavigateCommand}]" />

which looks bad for me, but I can't argue it. Is there any rules that I volatile? In any case, is there anything that can improve readability of this line?

Why?

If I want to add new functionality to my control, I create command class, inherit it from ExplorerCommand, decorate it with ExplorerCommandAttribute and I am done. CommandBindings are populated by retrieving all commands decorated by this attribute. I didn't change ViewModel for that. Also UI menu items are populated from this CommandBindings and there is no need to change UI too. This is flexibility that I achieved and won't lose it.

\$\endgroup\$
  • 1
    \$\begingroup\$ Why did you edit your code? \$\endgroup\$ – Zeta Sep 30 '17 at 8:06
  • \$\begingroup\$ I wrote a description. I am not supposed to do so ? P.S. I just removed passing variable to command, to not get confused by IExplorerModel that I didn't show in question. \$\endgroup\$ – Aram Kocharyan Sep 30 '17 at 8:18
  • 2
    \$\begingroup\$ I'm talking about changes like CommandBindings[nameof(NavigateCommand)].Execute(Model); to CommandBindings[nameof(NavigateCommand)].Execute(null);. If your question changes all the time, it will be hard to write a proper review. See also: What you may and may not do after receiving answers. \$\endgroup\$ – Zeta Sep 30 '17 at 8:21
  • \$\begingroup\$ Guys you think that passing Model to command, that isn't documented in question is more accurate than passing null to it ? Edit was done to simplify question and to not get confused what is that Model. \$\endgroup\$ – Aram Kocharyan Sep 30 '17 at 8:51
1
\$\begingroup\$

I might not fully understand how your commands are meant to be used, but the following things strike me as rather cumbersome:

  1. Requiring a separate class for each command.
  2. Command (class) names must be globally unique.
  3. Having to pass context to a command.

Alternative

An alternative is to create a single RelayCommand class that wraps an Action:

public class RelayCommand : ICommand
{
    public RelayCommand(Action action) { ... }
    ...
}

In your view models, you can expose commands as bindable properties:

public class MyViewModel
{
    private ICommand _saveDataCommand;
    public ICommand SaveDataCommand
    {
        get { return _saveDataCommand ?? (_saveDataCommand = new RelayCommand(SaveData)); }
    }

    private void SaveData()
    {
        ...
    }
}

And in your views, you can bind to these commands:

<Button Command="{Binding SaveDataCommand}" />

Now, you no longer have to create separate classes, just separate methods (and ICommand properties, but see below). Because those methods live in a specific view model, they likely already have access to the context they need, and their name only needs to be unique within that view model, not globally.

A possible further improvement

I've successfully been using the above approach in WPF, but having to create all those ICommand properties was still cumbersome. Initially I made a code snippet to make them easier to write, but eventually I settled on a different approach.

The data binding system in WPF (and perhaps also UWP, I'm not sure) not only uses reflection, it also checks if a data context implements ICustomTypeDescriptor, and if so, it'll use the GetProperties methods to detect additional 'properties'. I used that to create a view model base class that generates such a 'property' for each method that is marked with a custom [Command] attribute.

This took some work, but in the end it simplified things to just this:

public class MyViewModel : ViewModelBase
{
    [Command]
    public void SaveDataCommand()
    {
        ...
    }
}
\$\endgroup\$
  • \$\begingroup\$ Thank you for your time Pieter. Have you notices Why section of my question ? \$\endgroup\$ – Aram Kocharyan Sep 28 '17 at 11:12
  • \$\begingroup\$ Yes, I read that, but that didn't really clarify in what context those commands are meant to be used. Perhaps you could give a concrete example that shows the benefits of your approach? \$\endgroup\$ – Pieter Witvoet Sep 28 '17 at 11:21
  • \$\begingroup\$ With my described behavior, you can define your commands in separate assembly, and register them at runtime. So it means I have Control that can have custom commands, without touching ViewModel and XAML. So for adding command, you just add Command class and register it, nothing more. \$\endgroup\$ – Aram Kocharyan Sep 28 '17 at 17:54
  • \$\begingroup\$ It looks like we have different scenario's in mind, and that's ok, but I still don't see when the things you mention would actually be useful - and that makes it difficult to give a meaningful review. What sort of problem are you trying to solve? Why do you need to create commands in other assemblies? Why don't you want to modify your view models? Can you give a concrete example? \$\endgroup\$ – Pieter Witvoet Sep 28 '17 at 19:21
  • \$\begingroup\$ What sort of problem are you trying to solve? I have (will have) huge amount of commands like 20-30 and I think that writing 20-30 methods in one class will make it a little messy. Why do you need to create commands in other assemblies? First reason is that I wont to decouple them. Second is to allow writing extensions for app. \$\endgroup\$ – Aram Kocharyan Sep 28 '17 at 20:10

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.