8

A very common implementation of RelayCommand seems to include the following lines:

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

This seems very flawed to me, because CommandManager is a WPF component and usually my commands are located in a viewmodel class. Since the viewmodel is not supposed to know the view and should work with different frameworks and such, this seems very strange to me. For example this implementation will not even be possible if you separate your viewmodel in an extra project, that doesn't know the WPF namespace (e.g. PCL).

Is this implementation violating the MVVM pattern?
Or do you maybe place the RelayCommand in your view somehow?
If this is indeed flawed, is there a best practice implementation that solves this issue?

Tim Pohlmann
  • 4,140
  • 3
  • 32
  • 61
  • your viewmodel is for your view, just not referencing it. so its ok. – blindmeis Mar 10 '16 at 08:17
  • @blindmeis By using a view component it is indeed referencing it. It becomes pretty clear if you try to use this RelayCommand implementation in a non-wpf application. – Tim Pohlmann Mar 10 '16 at 08:18
  • i dont think that you will use a viemodel for wpf in another non-wpf application :) the main purpose is testing without UI and thats works. – blindmeis Mar 10 '16 at 08:21
  • @blindmeis One of the biggest advantages of MVVM is, that you can replace the view by any other view easily. – Tim Pohlmann Mar 10 '16 at 08:23
  • 1
    have you ever did this? replacing a wpf view with a non wpf view and using the same viewmodel? well at least its up to you. for my understanding its really ok to have ICommand Namespace in your viewmodel, but of course not reference to a UI/View element. i dont see replacing the view as the big advantage. the big advantage is Unit testing without UI. – blindmeis Mar 10 '16 at 08:24
  • @blindmeis I did not use MVVM a lot so far, but I am intending to do this, yes. – Tim Pohlmann Mar 10 '16 at 08:25

1 Answers1

3

This is an easy and quick & dirty implementation mostly only used for tutorial cases that do not tie the tutorial to a specific MVVM framework, but act as a generic do-it-yourself MVVM tutorial.

This approach - other than tight coupling - has several other disadvantages.

When the CommandManager.InvalidateRequerySuggested() method is called, the CanExecute method of every command will be called. If you have 100s of commands in your Application, this can have severe impact on performance of your WPF Application.

I personally always suggest using a matured MVVM Framework (Prism being my favorite for LoB applications). There the commands usually don't implement it this way but you call MyCommand.OnCanExecuteChanged() (in case of Prism) to trigger CanExecute validation for one single command.

If you have compound or multiple commands that depend on each other, you tie it yourself in code, i.e. by storing a list of related commands inside the view they are used and loop through it or registering their OnCanExecuteChanged() methods to a multicast delegate and calling that instead.

Is this implementation violating the MVVM pattern?

Technically yes.

Or do you maybe place the RelayCommand in your view somehow?

Not really, though you may be able to abstract it with an external factory, it do not seems to make sense (see the issue above)

If this is indeed flawed, is there a best practice implementation that solves this issue?

Don't have a global invalidation of commands state anyway. Tie the commands yourself that need their execution state tied together.

Tseng
  • 61,549
  • 15
  • 193
  • 205