Is this bad MVVM practice?

Question

Say I want to invoke a command in the view model using an event. Is this bad practice? If so why?

Example:

Code-behind

void button1_Click(object sender, MouseEventArgs e)
{
    this.ViewModel.OnButton1Click();
}

ViewModel

void OnButton1Click()
{
    // Do something
}

Thanks in advance.

Calling a method in your viewmodel is okay but it should call an action related to that viewmodel, not related to the UI. Why would your model care about buttons? — Jeroen Vannevel, Nov 24 '14 at 15:17
As Jeroen said, naming should be changed. The ViewModel should come before the View and the method should have a name reflecting the action. You probably should also consider binding the button to ICommand. — SKall, Nov 24 '14 at 15:25
Take a look: http://stackoverflow.com/questions/20883199/wpf-mvvm-code-behind — blfuentes, Nov 24 '14 at 15:32
`this.ViewModel` indicates issues. This isn't, otherwise, bad MVVM, but it may indicate your design has issues. UI logic is perfectly fine in your codebehind. Just keep your business logic out of it. — , Nov 24 '14 at 15:41

score 2 · Accepted Answer · answered Nov 24 '14 at 15:34

Yes it is bad practice, because you are directly referencing the ViewModel from the View which implies a dependency between the View and the ViewModel and thus tight coupling.

The pattern specifically calls for the View NOT to be dependent on a specific ViewModel instance or type. The idea here being decoupled Views and ViewModels for polymorphic Views (the idea that you could show the same data multiple ways). In order for your View to be reusable, it would need to not have a concrete dependency on a specific ViewModel type.

A better way to handle this would be to use Commands as Kamel says. However, not all events/interactions/controls necessarily allow Commands to be invoked. However, you can attach Commands to specific events using the System.Windows.Interactivity.dll assembly which comes with the Blend SDK.

This allows you to create XAML that looks like the following:

 <i:Interaction.Triggers>
      <i:EventTrigger EventName="LostFocus">
           <i:InvokeCommandAction
               Command="{Binding UpdateTextBoxBindingOnEnterCommand}"
               CommandParameter="{Binding RelativeSource={RelativeSource FindAncestor,AncestorType={x:Type TextBox}}}"/>
       </i:EventTrigger>
  </i:Interaction.Triggers>

..which allows the same functionality as an event handler to invoke a Command, but in a decoupled MVVM-friendly fashion.

As the others have said, yes this is bad practice. The ViewModel shouldn't be tied to the view further than setting the DataContext in the View Constructor. As the other answers suggest, simply implement the ICommand interface and use 'Command={Binding}' from your Xaml. — Hardgraf, Nov 25 '14 at 10:13

score 1 · Answer 2 · answered Nov 24 '14 at 15:25

MVVM in WPF is responsible for handling the application's presentation logic and state. this means that the view's code-behind file should contain no code to handle events that are raised from any UI. So what you are trying to do will break the MVVM

you should rewrite your code in something like the following

    <Button Command="{Binding ClickCommand}" Width="100" Height="100" Content="test"/>

the code behind for the window:

public partial class MainWindow : Window
{
    public MainWindow()
    {
        InitializeComponent();
        DataContext = new ViewModelBase();
    }
}

And the ViewModel:

public class ViewModelBase
{
    public ViewModelBase()
    {
        _canExecute = true;
    }
    private ICommand _clickCommand;
    public ICommand ClickCommand
    {
        get
        {
            return _clickCommand ?? (_clickCommand = new CommandHandler(() => MyAction(), _canExecute));
        }
    }
    private bool _canExecute;
    public void MyAction()
    {

    }
}
public class CommandHandler : ICommand
{
    private Action _action;
    private bool _canExecute;
    public CommandHandler(Action action, bool canExecute)
    {
        _action = action;
        _canExecute = canExecute;
    }

    public bool CanExecute(object parameter)
    {
        return _canExecute;
    }

    public event EventHandler CanExecuteChanged;

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

Is this bad MVVM practice?

2 Answers2