From event handler to command: on my way to MVVM

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)










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

///
/// Encapsulated the execute action
///
private Action execute;

///
/// Encapsulated the representation for the validation of the execute method
///
private Pred

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));

StackExchange Code Review Q#78238, answer score: 5