MVVM, do I have to keep each command in own class?

Question

I am trying to get used to MVVM and WPF for a month. I am trying to do some basic stuff, but I am constantly running into problems. I feel like I solved most of them by searching online. But now there comes the problem with Commands.

1. Q: I saw that they are using RelayCommand, DelegateCommand or SimpleCommand. Like this:

   public ICommand DeleteCommand => new SimpleCommand(DeleteProject);
   

Even though I create everything like they did, I am still having the part => new SimpleCommand(DeleteProject); redly underlined.

So far I am working around it by creating command class for every command, but that does not feel like the right way to go.

2. Q: I will also post whole project and I would like to know if I am doing anything wrong or what should I improve.

xaml:

<Window x:Class="WpfApplication1.MainWindow"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    Title="MainWindow" Height="380" Width="250">
<StackPanel DataContext="{Binding Source={StaticResource gallery}}" Margin="10">
    <ListView DataContext="{Binding Source={StaticResource viewModel}}" 
              SelectedItem="{Binding SelectedGallery}"
              ItemsSource="{Binding GalleryList}"
              Height="150">

        <ListView.View>
            <GridView>
                <GridViewColumn Header="Name" Width="100" DisplayMemberBinding="{Binding Name}"/>
                <GridViewColumn Header="Path" Width="100" DisplayMemberBinding="{Binding Path}"/>
            </GridView>
        </ListView.View>
    </ListView>
    <TextBlock Text="Name" Margin="0, 10, 0, 5"/>
    <TextBox Text="{Binding Name}" />
    <TextBlock Text="Path" Margin="0, 10, 0, 5" />
    <Grid>
        <Grid.ColumnDefinitions>
            <ColumnDefinition Width="*"/>
            <ColumnDefinition Width="40"/>
        </Grid.ColumnDefinitions>
        <TextBox Text="{Binding Path}" Grid.Column="0"/>
        <Button Command="{Binding Path=ShowFolderClick, Source={StaticResource viewModel}}"
                CommandParameter="{Binding}"
                Content="..." Grid.Column="1" Margin="10, 0, 0, 0"/>
    </Grid>

    <Grid Margin="0, 10, 0, 0">
        <Grid.ColumnDefinitions>
            <ColumnDefinition Width="*"/>
            <ColumnDefinition Width="*"/>
            <ColumnDefinition Width="*"/>
        </Grid.ColumnDefinitions>

        <Button Command="{Binding Path=AddClick, Source={StaticResource viewModel}}" 
                CommandParameter="{Binding}" Content="Add" Grid.Column="0" Margin="15,0,0,0" />
        <Button Command="{Binding Path=DeleteClick, Source={StaticResource viewModel}}"
                Content="Delete" Grid.Column="2" Margin="0,0,15,0" />
    </Grid>
</StackPanel>

Model:

class Gallery : INotifyPropertyChanged
{

    private string _name;
    public string Name
    {
        get
        {
            return _name;
        }
        set
        {
            _name = value;
            OnPropertyChanged("Name");
        }
    }


    private string _path;

    public string Path
    {
        get
        {
            return _path;
        }
        set
        {
            _path = value;
            OnPropertyChanged("Path");
        }
    }

    public event PropertyChangedEventHandler PropertyChanged;


    private void OnPropertyChanged(params string[] propertyNames)
    {
        PropertyChangedEventHandler handler = PropertyChanged;

        if (handler != null)
        {
            foreach (string propertyName in propertyNames) PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
            handler(this, new PropertyChangedEventArgs("HasError"));
        }
    }
}

ModelView:

class GalleryViewModel : INotifyPropertyChanged
{
    public GalleryViewModel()
    {
        GalleryList = new ObservableCollection<Gallery>();
        this.ShowFolderClick = new ShowFolderDialog(this);
        this.AddClick = new AddGalleryCommand(this);
        this.DeleteClick = new DeleteGalleryCommand(this);
    }

    private ObservableCollection<Gallery> _galleryList;

    public ObservableCollection<Gallery> GalleryList
    {
        get { return _galleryList; }
        set { 
            _galleryList = value;
            OnPropertyChanged("GalleryList");
        }
    }

    private Gallery _selectedGallery;

    public Gallery SelectedGallery
    {
        get { return _selectedGallery; }
        set { 
            _selectedGallery = value;
            OnPropertyChanged("SelectedGallery");
        }
    }

    public event PropertyChangedEventHandler PropertyChanged;
    private void OnPropertyChanged(params string[] propertyNames)
    {
        PropertyChangedEventHandler handler = PropertyChanged;

        if (handler != null)
        {
            foreach (string propertyName in propertyNames) PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
            handler(this, new PropertyChangedEventArgs("HasError"));
        }
    }

    public AddGalleryCommand AddClick { get; set; }
    public void AddGalleryClick(Gallery gallery)
    {

        Gallery g = new Gallery();
        g.Name = gallery.Name;
        g.Path = gallery.Path;
        GalleryList.Add(g);

    }

    public DeleteGalleryCommand DeleteClick { get; set; }
    public void DeleteGalleryClick()
    {
        if (SelectedGallery != null)
        {
            GalleryList.Remove(SelectedGallery);
        }
    }

    public ShowFolderDialog ShowFolderClick { get; set; }
    public void ShowFolderDialogClick(Gallery gallery)
    {
        System.Windows.Forms.FolderBrowserDialog browser = new System.Windows.Forms.FolderBrowserDialog();
        string tempPath = "";

        if (browser.ShowDialog() == System.Windows.Forms.DialogResult.OK)
        {
            tempPath = browser.SelectedPath; // prints path
        }

        gallery.Path = tempPath;
    }
}

Commands:

class AddGalleryCommand : ICommand
{
    public GalleryViewModel _viewModel { get; set; }

    public AddGalleryCommand(GalleryViewModel ViewModel)
    {
        this._viewModel = ViewModel;
    }

    public bool CanExecute(object parameter)
    {
        /*if (parameter == null)
            return false;*/
        return true;
    }

    public event EventHandler CanExecuteChanged;

    public void Execute(object parameter)
    {
        this._viewModel.AddGalleryClick(parameter as Gallery);
    }
}

class DeleteGalleryCommand : ICommand
{
    public GalleryViewModel _viewModel { get; set; }

    public DeleteGalleryCommand(GalleryViewModel ViewModel)
    {
        this._viewModel = ViewModel;
    }
    public bool CanExecute(object parameter)
    {
        return true;
    }

    public event EventHandler CanExecuteChanged;

    public void Execute(object parameter)
    {
        this._viewModel.DeleteGalleryClick();
    }
}

class ShowFolderDialog : ICommand
{
    public GalleryViewModel _viewModel { get; set; }

    public ShowFolderDialog(GalleryViewModel ViewModel)
    {
        this._viewModel = ViewModel;
    }
    public bool CanExecute(object parameter)
    {
        return true;
    }

    public event EventHandler CanExecuteChanged;

    public void Execute(object parameter)
    {
        this._viewModel.ShowFolderDialogClick(parameter as Gallery);
    }
}

Thanks for your time reading so far, I will appreciate every advice I will get.

Answer 1

There are frameworks/library that help with simplifying command binding. For example MVVMLight has generic implementation of RelayCommand that only needs you to create property and assign method name for it execute.

Here's an example of how Mvvmlight Relaycommand is used.

Answer 2

You can do this by using a single DelegateCommand implementation rather than separate ICommand classes.

public class DelegateCommand : ICommand
{
    private readonly Predicate<object> _canExecute;
    private readonly Action<object> _execute;

    public event EventHandler CanExecuteChanged;

    public DelegateCommand(Action<object> execute, Predicate<object> canExecute)
    {
        _execute = execute;
        _canExecute = canExecute;
    }

    public DelegateCommand(Action<object> execute) : this(execute, null) { }

    public virtual bool CanExecute(object parameter)
    {
        if (_canExecute == null)
        {
            return true;
        }

        return _canExecute(parameter);
    }

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

    public void RaiseCanExecuteChanged()
    {
        CanExecuteChanged?.Invoke(this, EventArgs.Empty);
    }
}

There are two overloaded constructors, one accepting only the method to execute, and one accepting both the method and a Predicate for CanExecute.

Usage:

public class ViewModel
{
    public ICommand DeleteProjectCommand => new DelegateCommand(DeleteProject);

    private void DeleteProject(object parameter)
    {
    }
}

With regards to further simplification along the lines of MVVM, one way to implement property change notification functionality is via the following:

public abstract class ObservableObject : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;

    internal void NotifyPropertyChanged([CallerMemberName] string propertyName = "")
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
    }
}

And then in the ViewModel:

public class ViewModel : ObservableObject
{
    private object _myProperty;
    public object MyProperty
    {
        get { return _myProperty; }
        set
        {
            if (_myProperty != value)
            {
                _myProperty = value;
                NotifyPropertyChanged();
            }
        }
    }

    private object _anotherProperty;
    public object AnotherProperty
    {
        get { return _anotherProperty; }
        set
        {
            if (_anotherProperty != value)
            {
                _anotherProperty = value;
                NotifyPropertyChanged();
                NotifyPropertyChanged("MyProperty");
            }
        }
    }
}

Notice that you don't need to provide the name of the property when raising NotifyPropertyChanged from within that property's setter (thanks to [CallerMemberName]), although it's still an option to do so, e.g., AnotherProperty raises change notifications for both properties.

Clarification

DelegateCommand will work for all of your examples. The method you pass to it should have a signature of:

void MethodName(object parameter)

This matches the signature of the Execute method of ICommand. The parameter type is object, so it accepts anything, and within your method you can cast it as whatever object you've actually passed to it, e.g.:

private void AddGallery(object parameter)
{
    Gallery gallery = (Gallery)parameter;

    ...
}

If you set no CommandParameter, then null will be passed, so for your other example, you can still use the same signature, you just won't use the parameter:

private void DeleteGallery(object parameter)
{
    ...
}

So you can use a DelegateCommand for all of the above.

CanAddGallery Implementation

The following should provide a good model for how to implement this (I've invented two properties, Property1 and Property2, to represent your TextBox values):

public class Gallery : ObservableObject
{
    private string _property1;
    public Gallery Property1
    {
        get { return _property1; }
        set
        {
            if (_property1 != value)
            {
                _property1 = value;
                NotifyPropertyChanged();
            }
        }
    }

    private Gallery _property2;
    public Gallery Property2
    {
        get { return _property2; }
        set
        {
            if (_property2 != value)
            {
                _property2 = value;
                NotifyPropertyChanged();
            }
        }
    }

    public Gallery() { }
}

public class AddGalleryViewModel : ObservableObject
{
    private Gallery _galleryToAdd;
    public Gallery GalleryToAdd
    {
        get { return _galleryToAdd; }
        set
        {
            if (_galleryToAdd != value)
            {
                _galleryToAdd = value;
                NotifyPropertyChanged();
            }
        }
    }

    public DelegateCommand AddGalleryCommand { get; set; }

    public AddGalleryViewModel()
    {
        AddGalleryCommand = new DelegateCommand(AddGallery, CanAddGallery)

        GalleryToAdd = new Gallery();
        GalleryToAdd.PropertyChanged += GalleryToAdd_PropertyChanged
    }

    private void AddGallery(object parameter)
    {
        Gallery gallery = (Gallery)parameter;

        ...
    }

    private bool CanAddGallery(object parameter)
    {
        Gallery gallery = (Gallery)parameter;

        if (string.IsNullOrEmpty(gallery.Property1) || string.IsNullOrEmpty(gallery.Property2))
        {
            return false;
        }

        return true;
    }

    private void GalleryToAdd_PropertyChanged(object sender, PropertyChangedEventArgs e)
    {
        if (e.PropertyName == "Property1" || e.PropertyName == "Property2")
        {
            AddGalleryCommand.RaiseCanExecuteChanged();
        }
    }
}

A note on the following implementation:

public DelegateCommand AddGalleryCommand => new DelegateCommand(AddGallery, CanAddGallery);

I find that when I use this method, the CanExecuteChanged EventHandler on the DelegateCommand is always null, and so the event never fires. If CanExecute is false to begin with, the button will always be disabled - if it's true to begin with, I still get accurate functionality in terms of the command executing or not, but the button will always be enabled. Therefore, I prefer the method in the above example, i.e.:

public DelegateCommand AddGalleryCommand { get; set; }

public AddGalleryViewModel()
{
    AddGalleryCommand = new DelegateCommand(AddGallery, CanAddGallery)

    ...
}

DelegateCommand Specialisations

The following class allows you to specify a type for your command parameter:

public class DelegateCommand<T> : ICommand
{
    private readonly Predicate<T> _canExecute;
    private readonly Action<T> _execute;

    public event EventHandler CanExecuteChanged;

    public DelegateCommand(Action<T> execute, Predicate<T> canExecute)
    {
        _execute = execute;
        _canExecute = canExecute;
    }

    public DelegateCommand(Action<T> execute) : this(execute, null) { }

    public virtual bool CanExecute(object parameter)
    {
        if (_canExecute == null)
        {
            return true;
        }

        return _canExecute((T)parameter);
    }

    public void Execute(object parameter)
    {
        _execute((T)parameter);
    }

    public void RaiseCanExecuteChanged()
    {
        CanExecuteChanged?.Invoke(this, EventArgs.Empty);
    }
}

Usage:

public DelegateCommand<Gallery> AddGalleryCommand { get; set; }

public AddGalleryViewModel()
{
    AddGalleryCommand = new DelegateCommand<Gallery>(AddGallery, CanAddGallery)
}

private void AddGallery(Gallery gallery)
{
    ...
}

private bool CanAddGallery(Gallery gallery)
{
    ...
}

The following allows you to specify a parameterless method:

public delegate void ParameterlessAction();
public delegate bool ParameterlessPredicate();

public class InternalDelegateCommand : ICommand
{
    private readonly ParameterlessPredicate _canExecute;
    private readonly ParameterlessAction _execute;

    public event EventHandler CanExecuteChanged;

    public InternalDelegateCommand(ParameterlessAction execute) : this(execute, null) { }

    public InternalDelegateCommand(ParameterlessAction execute, ParameterlessPredicate canExecute)
    {
        _execute = execute;
        _canExecute = canExecute;
    }

    public bool CanExecute(object parameter)
    {
        if (_canExecute == null)
        {
            return true;
        }

        return _canExecute();
    }

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

    public void RaiseCanExecuteChanged()
    {
        CanExecuteChanged?.Invoke(this, EventArgs.Empty);
    }
}

Usage:

public InternalDelegateCommand CreateGalleryCommand { get; set; }

public CreateGalleryViewModel()
{
    CreateGalleryCommand = new InternalDelegateCommand(CreateGallery)
}

private void CreateGallery()
{
    Gallery gallery = new Gallery();

    ...
}

Answer 3

Ok, I have tried to simplify it as much as possible. I am using yours ObservableObject and DelegateCommand<T>.

Problem is NullReferenceException in CanAddGallery right after run. No window pop up. I tried to solve it by adding if (parameter == null) return false. That will just disable the button. I am thinking if it is necessary to disable button. If wouldn't it be better from the user's point of view insted of disabling button, to have red text under textbox saying "this must be filled" (or pop up message), that will appear when no parameters are send through button click.

xaml:

<StackPanel DataContext="{Binding Source={StaticResource gallery}}" Margin="10">
    <ListView DataContext="{Binding Source={StaticResource viewModel}}" 
              SelectedItem="{Binding SelectedGallery}"
              ItemsSource="{Binding GalleryList}"
              Height="150">

        <ListView.View>
            <GridView>
                <GridViewColumn Header="Name" Width="100" DisplayMemberBinding="{Binding Name}"/>
                <GridViewColumn Header="Path" Width="100" DisplayMemberBinding="{Binding Path}"/>
            </GridView>
        </ListView.View>
    </ListView>

    <TextBlock Text="Name" Margin="0, 10, 0, 5"/>
    <TextBox Text="{Binding Name}" />
    <TextBlock Text="Path" Margin="0, 10, 0, 5" />
    <TextBox Text="{Binding Path}" Grid.Column="0"/>
    <Button DataContext="{Binding Source={StaticResource viewModel}}"
            Command="{Binding Path=AddGalleryCommand}" 
            CommandParameter="{Binding Path=GalleryToAdd}" Content="Add"/>
</StackPanel>

Model:

class Gallery : ObservableObject
{
    private string _name;
    public string Name
    {
        get
        {
            return _name;
        }
        set
        {
            _name = value;
            NotifyPropertyChanged();
        }
    }


    private string _path;

    public string Path
    {
        get
        {
            return _path;
        }
        set
        {
            _path = value;
            NotifyPropertyChanged();
        }
    }
}

ViewModel:

class GalleryViewModel : ObservableObject
{
    private ObservableCollection<Gallery> _galleryList;

    public ObservableCollection<Gallery> GalleryList
    {
        get { return _galleryList; }
        set
        {
            _galleryList = value;
            NotifyPropertyChanged();
        }
    }

    private Gallery _galleryToAdd;
    public Gallery GalleryToAdd
    {
        get { return _galleryToAdd; }
        set
        {
            if (_galleryToAdd != value)
            {
                _galleryToAdd = value;
                NotifyPropertyChanged();
            }
        }
    }

    public DelegateCommand<Gallery> AddGalleryCommand { get; set; }

    public GalleryViewModel()
    {
        GalleryList = new ObservableCollection<Gallery>();
        AddGalleryCommand = new DelegateCommand<Gallery>(AddGallery, CanAddGallery);
        GalleryToAdd = new Gallery();
        GalleryToAdd.PropertyChanged += GalleryToAdd_PropertyChanged;
    }

    private void AddGallery(object parameter)
    {
        Gallery gallery = (Gallery)parameter;

        Gallery g = new Gallery();
        g.Name = gallery.Name;
        g.Path = gallery.Path;
        GalleryList.Add(g);
    }

    private bool CanAddGallery(object parameter)
    {
        Gallery gallery = (Gallery)parameter;

        if (string.IsNullOrEmpty(gallery.Name) || string.IsNullOrEmpty(gallery.Path))
        {
            return false;
        }

        return true;
    }

    private void GalleryToAdd_PropertyChanged(object sender, PropertyChangedEventArgs e)
    {
        if (e.PropertyName == "Name" || e.PropertyName == "Path")
        {
            AddGalleryCommand.RaiseCanExecuteChanged();
        }
    }
}