Simplifying RelayCommand/DelegateCommand in WPF MVVM ViewModels

Question

If you're doing MVVM and using commands, you'll often see ICommand properties on the ViewModel that are backed by private RelayCommand or DelegateCommand fields, like this example from the original MVVM article on MSDN:

RelayCommand _saveCommand;
public ICommand SaveCommand
{
    get
    {
        if (_saveCommand == null)
        {
            _saveCommand = new RelayCommand(param => this.Save(),
                param => this.CanSave );
        }
        return _saveCommand;
    }
}

However, this is a lot of clutter, and makes setting up new commands rather tedious (I work with some veteran WinForms developers who balk at all this typing). So I wanted to simplify it and dug in a little. I set a breakpoint at the first line of the get{} block and saw that it only got hit when my app was first loaded--I can later fire off as many commands as I want and this breakpoint never gets hit--so I wanted to simplify this to remove some clutter from my ViewModels and noticed that the following code works the same:

public ICommand SaveCommand
{
    get
    {
        return new RelayCommand(param => this.Save(), param => this.CanSave );
    }
}

However, I don't know enough about C# or the garbage collector to know if this could cause problems, such as generating excessive garbage in some cases. Will this pose any problems?

Answer 1

I discovered that you need the original way from MSDN if you have multiple controls that invoke the same commands, otherwise each control will new its own RelayCommand. I didn't realize this because my app only has one control per command.

So to simplify the code in ViewModels, I'll create a command wrapper class that stores (and lazily instantiates) all the RelayCommands and throw it in my ViewModelBase class. This way users do not have to directly instantiate RelayCommand or DelegateCommand objects and don't need to know anything about them:

    /// <summary>
    /// Wrapper for command objects, created for convenience to simplify ViewModel code
    /// </summary>
    /// <author>Ben Schoepke</author>
    public class CommandWrapper
    {
    private readonly List<DelegateCommand<object>> _commands; // cache all commands as needed

    /// <summary>
    /// </summary>
    public CommandWrapper()
    {
        _commands = new List<DelegateCommand<object>>();
    }

    /// <summary>
    /// Returns the ICommand object that contains the given delegates
    /// </summary>
    /// <param name="executeMethod">Defines the method to be called when the command is invoked</param>
    /// <param name="canExecuteMethod">Defines the method that determines whether the command can execute in its current state.
    /// Pass null if the command should always be executed.</param>
    /// <returns>The ICommand object that contains the given delegates</returns>
    /// <author>Ben Schoepke</author>
    public ICommand GetCommand(Action<object> executeMethod, Predicate<object> canExecuteMethod)
    {
        // Search for command in list of commands
        var command = (_commands.Where(
                            cachedCommand => cachedCommand.ExecuteMethod.Equals(executeMethod) &&
                                             cachedCommand.CanExecuteMethod.Equals(canExecuteMethod)))
                                             .FirstOrDefault();

        // If command is found, return it
        if (command != null)
        {
            return command;
        }

        // If command is not found, add it to the list
        command = new DelegateCommand<object>(executeMethod, canExecuteMethod);
        _commands.Add(command);
        return command;
    }
}

This class is also lazily instantiated by the ViewModelBase class, so ViewModels that do not have any commands will avoid the extra allocations.

Answer 2

One thing that I do is let Visual Studio do the typing for me. I just created a code snippet that allows me to create a RelayCommand by typing

rc Tab Save Enter

rc is the code snippet shortcut tab loads the text you type what you want and it creates all the other wording.

Once you look at one code snippet and create your own you'll never go back :)

Answer 3

This is exactly the same as if you would offer a - say integer - property that calculates some constant value. You can either calculate it for each call on the get-method or you can create it on the first call and then cache it, in order to return the cached value for later calls. So if the getter is called at most once, it does make no difference at all, if it is called often, you will lose some (not much) performance, but you won't get real trouble.

I personally like to abbreviate the MSDN-way like this:

RelayCommand _saveCommand;
public ICommand SaveCommand
{
  get
  {
    return _saveCommand ?? (_saveCommand = new RelayCommand(param => this.Save(),
                                                            param => this.CanSave ));
  }
}

Answer 4

Why don't you write just:

private readonly RelayCommand _saveCommand = new RelayCommand(param => this.Save(),
                param => this.CanSave );;

public ICommand SaveCommand { get { return _saveCommand; } }