RelayCommand best practice

Question

I'm working with RelayCommands (they are in a separate class) for about a month now and I got the feeling they're kind of clunky when declaring them. Below I have 3 ways I can think of how I can declare a RelayCommand.

In a first case I declare my ICommand and then when the ViewModel is loading I construct my RelayCommand that points to a method in my code.

public class MyViewModel
{
    public ICommand MyCommand { get; private set; }

    public MyViewModel()
    {
        MyCommand = new RelayCommand(MyMethod, CanMyMethod);
    }

    private void MyMethod()
    {
         // Do something here ...
    }

    private bool CanMyMethod()
    {
         return string.IsNullOrEmpty(MyString) ? false : true;
    }
}

A second method is to do everything at once.

public ICommand MyCommand
    {
        get
        {
            return new RelayCommand(
                () =>
                {
                    // Do something here ...
                },
                () =>
                    string.IsNullOrEmpty(MyString) ? false : true);
        }
    }

Now, I'm planning to write an application with quite some Commands in a certain ViewModel. I also can't split the ViewModel in smaller ViewModels because all the controls have to work together.

So my questions are:

1. What is the best approach to declaring and constructing ICommands? Is it one of my approaches or is there an easier way?
2. How hard is it to maintain the overview with each approach considering there are over 50 ICommands in a single ViewModel.
3. I'm hoping to release my application on both Windows 7, 8 and 10 in the future. Are there any limitations to RelayCommands I have to take in account if I'm only using .NET4.5?
4. Besides RelayCommands I also found this project: Caliburn-Micro. It allows you to do something like the code below. Does anyone have an idea how good this works performance wise in comparison to RelayCommands? This is just an extra question and not required to be answered to have a post marked as an answer.

Xaml (View)

<Button x:Name="Login" Content="Log in" />

ViewModel

public bool CanLogin(string username, string password)
{
    return !String.IsNullOrEmpty(username) && !String.IsNullOrEmpty(password);
}

public string Login(string username, string password)
{
    ...
}

Answer 1

Refer below answers.

1. What is the best approach to declaring and constructing ICommands? Is it one of my approaches or is there an easier way? Ans:You can take combined approach. If your execute method is very minimal, you can use RelayCommnd otherwise you can implement your own ICommand in a separate class. This will improve readability of your viewmodel as well as modularity of your code.
2. How hard is it to maintain the overview with each approach considering there are over 50 ICommands in a single ViewModel. Ans: Cover in Ans 1
3. I'm hoping to release my application on both Windows 7, 8 and 10 in the future. Are there any limitations to RelayCommands I have to take in account if I'm only using .NET4.5? Ans: I am not seeing any limitation in Windows 7 or 8 but not sure about Windows 10.
4. Besides RelayCommands I also found this project: Caliburn-Micro. It allows you to do something like the code below. Does anyone have an idea how good this works performance wise in comparison to RelayCommands? This is just an extra question and not required to be answered to have a post marked as an answer. Ans: I am not sure about source code of RelayCommands in Caliburn-Micro. But if it using CommandManager to achieve the CanExecute functionality. Then command manager will get triggered for all the user inputs change and hence that will cause performance issues if have some heavy logic in the CanExecute method. Refer Are there any performance implications with CanExecuteCommand?

Answer 2

Here's the pattern that I prefer, which is basically a variation of method 1:

public class MyViewModel
{
    private readonly RelayCommand myCommand;

    public MyViewModel()
    {
        this.myCommand = new RelayCommand(this.DoStuff, () => ...);
    }

    public ICommand MyCommand
    {
        get { return this.myCommand; }
    }

    private void DoStuff() { ... }
}

This has the advantage of keeping extra methods on your RelayCommand implementation (like RaiseCanExecuteChanged) around for use from your view model, but exposes only an ICommand instance to consumers.

Answer 3

I agree with Krowi, the first approach is much easier to read. But I would put your relay command into its own class so you can re-use it:

public class RelayCommand : ICommand
{
    #region Fields
    readonly Action<object> _execute;
    readonly Predicate<object> _canExecute;
    #endregion // Fields

    #region Constructors
    public RelayCommand(Action<object> execute)
        : this(execute, null)
    {
    }

    public RelayCommand(Action<object> execute, Predicate<object> canExecute)
    {
        if (execute == null)
            throw new ArgumentNullException("execute");

        _execute = execute;
        _canExecute = canExecute;
    }
    #endregion

    #region ICommand Members
    [DebuggerStepThrough]
    public bool CanExecute(object parameter)
    {
        return _canExecute == null ? true : _canExecute(parameter);
    }

    public event EventHandler CanExecuteChanged
    {
        add { CommandManager.RequerySuggested += value; }
        remove { CommandManager.RequerySuggested -= value; }
    }

    public void Execute(object parameter)
    {
        _execute(parameter);
    }
    #endregion
}