From event handler to command: on my way to MVVM

Question

I have a WPF C# application that I want to move to a MVVM pattern.

My first step is to remove the button click handlers from the code behind of the UI.

I've found some examples online and then modified them to my needs. Is the following code a good way to use buttons without the code-behind click handlers? And is this actually considered MVVM?

SmallWindow.xaml (UI)

<Window
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    x:Class="Standards_convertor.SmallWindow"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:local="clr-namespace:convertor"
    Closing="OnClose" StateChanged="OnStateChanged" IsVisibleChanged="OnIsVisibleChanged"
Title="DVNR Software" Height="662.732" Width="983.806" ResizeMode="CanMinimize" BorderBrush="Black"
Style="{StaticResource DVNRStyle}">
<Window.DataContext>
    <local:ViewModels/>
</Window.DataContext>

<Grid Width="982" Margin="0,0,0,1">
<Button Content="Preset Editor" HorizontalAlignment="Left" Margin="45,456,0,0" VerticalAlignment="Top" Width="179" Command="{Binding OpenPresetEditorCommand}" Background="#FF658777" Foreground="White" Height="33" FontWeight="Bold" BorderBrush="{x:Null}" FontFamily="Segoe UI Light" FontSize="16" BorderThickness="0" Style="{StaticResource {x:Static ToolBar.ButtonStyleKey}}" Focusable="False"/>
    <Button x:Name="BtnPresetEditor" Content="Watchfolder Editor" HorizontalAlignment="Left" Margin="45,509,0,0" VerticalAlignment="Top" Width="179" Command="{Binding OpenWFEditorCommand}" Background="#FF6778A6" Foreground="White" Height="33" FontWeight="Bold" BorderBrush="{x:Null}" FontFamily="Segoe UI Light" FontSize="16" BorderThickness="0" Style="{StaticResource {x:Static ToolBar.ButtonStyleKey}}" Focusable="False"/>
    <Button Content="Settings" HorizontalAlignment="Left" Margin="45,562,0,0" VerticalAlignment="Top" Width="179" Command="{Binding OpenSettingsCommand}" Background="#FF6C6C6C" Foreground="White" Height="33" FontWeight="Bold" BorderBrush="{x:Null}" FontFamily="Segoe UI Light" FontSize="16" BorderThickness="0" Style="{StaticResource {x:Static ToolBar.ButtonStyleKey}}" Focusable="False"/>  
</Grid>
</Window>

ViewModel.cs

 class ViewModels
{
    Processing processing = new Processing();

    private bool canExecute = true;
           private ICommand openSettingsCommand;
    public ICommand OpenSettingsCommand
    {
        get { return openSettingsCommand; }
        set { openSettingsCommand = value; }
    }

    private ICommand openWFEditorCommand;
    public ICommand OpenWFEditorCommand
    {
        get { return openWFEditorCommand; }
        set { openWFEditorCommand = value; }
    }
    private ICommand openPresetEditorCommand;
    public ICommand OpenPresetEditorCommand
    {
        get { return openPresetEditorCommand; }
        set { openPresetEditorCommand = value; }
    }

    public ViewModels()
    {
        OpenSettingsCommand = new RelayCommand(OpenWindow, param => this.canExecute);
        OpenWFEditorCommand = new RelayCommand(OpenWFEditorWindow, param => this.canExecute);
        OpenPresetEditorCommand = new RelayCommand(OpenWFEditorWindow, param => this.canExecute);
    }

    public void OpenWindow(object obj)
    {
        processing.OpenSettings();
    }

    public void OpenWFEditorWindow(object obj)
    {
        processing.OpenWFEditor();
    }

    public void OpenPresetEditorWindow(object obj)
    {
        processing.OpenPresetEditor();
    }
}


//reuseable relay command used for wpf icommand
public class RelayCommand : ICommand
{
    #region Fields

    /// <summary>
    /// Encapsulated the execute action
    /// </summary>
    private Action<object> execute;

    /// <summary>
    /// Encapsulated the representation for the validation of the execute method
    /// </summary>
    private Predicate<object> canExecute;

    #endregion // Fields

    #region Constructors

    /// <summary>
    /// Initializes a new instance of the RelayCommand class
    /// Creates a new command that can always execute.
    /// </summary>
    /// <param name="execute">The execution logic.</param>
    public RelayCommand(Action<object> execute)
        : this(execute, DefaultCanExecute)
    {
    }

    /// <summary>
    /// Initializes a new instance of the RelayCommand class
    /// Creates a new command.
    /// </summary>
    /// <param name="execute">The execution logic.</param>
    /// <param name="canExecute">The execution status logic.</param>
    public RelayCommand(Action<object> execute, Predicate<object> canExecute)
    {
        if (execute == null)
        {
            throw new ArgumentNullException("execute");
        }

        if (canExecute == null)
        {
            throw new ArgumentNullException("canExecute");
        }

        this.execute = execute;
        this.canExecute = canExecute;
    }

    #endregion // Constructors

    #region ICommand Members

    /// <summary>
    /// An event to raise when the CanExecute value is changed
    /// </summary>
    /// <remarks>
    /// Any subscription to this event will automatically subscribe to both 
    /// the local OnCanExecuteChanged method AND
    /// the CommandManager RequerySuggested event
    /// </remarks>
    public event EventHandler CanExecuteChanged
    {
        add
        {
            CommandManager.RequerySuggested += value;
            this.CanExecuteChangedInternal += value;
        }

        remove
        {
            CommandManager.RequerySuggested -= value;
            this.CanExecuteChangedInternal -= value;
        }
    }

    /// <summary>
    /// An event to allow the CanExecuteChanged event to be raised manually
    /// </summary>
    private event EventHandler CanExecuteChangedInternal;

    /// <summary>
    /// Defines if command can be executed
    /// </summary>
    /// <param name="parameter">the parameter that represents the validation method</param>
    /// <returns>true if the command can be executed</returns>
    public bool CanExecute(object parameter)
    {
        return this.canExecute != null && this.canExecute(parameter);
    }

    /// <summary>
    /// Execute the encapsulated command
    /// </summary>
    /// <param name="parameter">the parameter that represents the execution method</param>
    public void Execute(object parameter)
    {
        this.execute(parameter);
    }

    #endregion // ICommand Members

    /// <summary>
    /// Raises the can execute changed.
    /// </summary>
    public void OnCanExecuteChanged()
    {
        EventHandler handler = this.CanExecuteChangedInternal;
        if (handler != null)
        {
            //DispatcherHelper.BeginInvokeOnUIThread(() => handler.Invoke(this, EventArgs.Empty));
            handler.Invoke(this, EventArgs.Empty);
        }
    }

    /// <summary>
    /// Destroys this instance.
    /// </summary>
    public void Destroy()
    {
        this.canExecute = _ => false;
        this.execute = _ => { return; };
    }

    /// <summary>
    /// Defines if command can be executed (default behaviour)
    /// </summary>
    /// <param name="parameter">The parameter.</param>
    /// <returns>Always true</returns>
    private static bool DefaultCanExecute(object parameter)
    {
        return true;
    }
}

Processing.cs 'Business logic'

public void OpenSettings()
    {
        Settings settings = new Settings();
        settings.Show();
    }

    public void OpenWFEditor()
    {
        WatchfolderEditor wfEditor = new WatchfolderEditor();
        wfEditor.Show();
    }

    public void OpenPresetEditor()
    {
        PresetEditor presetEditor = new PresetEditor();
        presetEditor.Show();
    }

Answer 1

MVVM

And is this actually considered MVVM?

I would say parts of your implementations are parts of MVVM.

• View
  Is represented by SmallWindow.xaml and is decoupled from the code behind and any business logic.
  Checked.

• ViewModel
  Is represented by ViewModels and provides the binding and presentation logic for the View.
  Checked.

• Model
  Processing is displaying views, but this isn't what a Model is about.
  The Model is holding your data. It provides methods to request data, which are delivered using events. It also provides methods to update/add/delete data.

Also MVVM is Model-View-ViewModel, there can be Controller(s) involved. See here.

In my understanding you should provide for each of the views (Settings, WatchfolderEditor,PresetEditor) either individual ViewModel's or put the presentation logic and binding into your ViewModels class. If you use individual ViewModel's you need to introduce some kind of Controller which controls their behaviour.

ViewModel.cs

• because this should represent a (one) ViewModel, you should rename the class from ViewModels to ViewModel.

• you should use autoimplemented properties instead of using private backing fields. This will reduce the amount of code and improves readability. Until you are going to do some validation in the setters or calculation in the getters you should always use them.

RelayCommand

• the CanExecute() can be simplified, because you are validating at constructor level if this.canExecute != null.

  public bool CanExecute(object parameter)
  {
      return this.canExecute != null && this.canExecute(parameter);
  }  
  

  can be simplified to

  public bool CanExecute(object parameter)
  {
      return this.canExecute(parameter);
  }  
  

• the scope of the OnCanExecuteChanged() method needs to be reduced to protected. Why would you want that the event is beeing raised from outside of the class ?

General

• commented code is dead code. Dead code should be deleted.

  //DispatcherHelper.BeginInvokeOnUIThread(() => handler.Invoke(this, EventArgs.Empty));