Should the viewmodel be opening windows?

Jan 8, 2015 at 8:54 AM
I see a lot of people saying that if you want to open a child window, the button on your view should bind to a vm command, and that command should send a message that the view picks up.

Isn't this an overly complex way of doing it? The view is telling the vm, "please instruct me to open a window," which seems pointless. The view already knows it needs to do this, so why bother involving the vm?

Apart from the obvious problem of extra complexity, you're adding more code to the view, as it has to register/deregister for messages, pick up incoming messages, check it's relevant and only then open the window. Something that can be done with one or two lines of view code becomes a convoluted trail of extra code in both view and vm.

Furthermore, if I come to look at someone's code, and want to follow the path of execution, I have to find all places where this message type is used, then see which of these is the one I want. If we use a new message type for every situation, we end up with an explosion of types, whereas if we reuse them, we obfuscate the code("now, which of these 27 methods that picks up FerretMessage is the one I want?"), and make it much harder to follow what's going on, and increasing the chance of bugs when something else picks up on the message.

Messages are great when you want to broadcast something, and you don't know (or care) who picks up the message, but using one in this case seems totally the wrong thing to do.

Don't get me wrong, I'm all for keeping code out of the view where possible, but here you're adding more code to the view, and needlessly involving the view model.

Can someone explain the point of doing it this way, as it seems wrong to me.
Coordinator
Jan 8, 2015 at 9:00 AM
Hi,

I completely agree with you. Only put in the VM what should be tested by unit tests, shared with other projects, etc. If you have some view-only code, it is perfectly OK to leave it in the view only. If you need to compute some condition deciding if the child window should be opened or not, it is OK to create a method doing the calculation and returning a value, and to have the view call this method.

Cheers
Laurent
Jan 8, 2015 at 2:02 PM
Thanks for that. Maybe I can extend this and ask a related question, as we have been debating this issue in our team.

Suppose you have a window showing details of a sales quote (for example), and you want to allow the user to click a button to open a child window to pick the customer. I am in favour of doing something like this in the button's click event handler...
private void PickCustomer_Click(object sender, RoutedEventArgs e) {
  PickCustomerWindow pcw = new PickCustomerWindow();
  pcw.Closing += (_, __) => {
    if (pcw.Customer != null) {
      ((QuoteDetailsViewModel)DataContext).SetCustomer(pcw.Customer);
    }
  };
  pcw.ShowDialog(this);
}
The PickCustomerWindow window has a Customer property, which does nothing more than return the view model's SelectedCustomer property.

The reason I like this approach is that I can see immediately what's happening when the user clicks the button. The window is opened, and then when it closes, if they have picked a customer, that is passed down to the vm, where it will be set on the underlying entity, and the changes reflected in the view due to the binding being updated. If I want to see what happens in the VM when a customer is picked, I can put my cursor on the SetCustomer() method and click f12 (gotta love Visual Studio's shortcuts!), and I am taken right to the code.

You could simplify this even more by removing the null check, and doing that in the vm. That makes the view code even simpler, although I'm not sure the benefit is particularly great.

The other view is to have the PickCustomerWindow send out a message saying a customer has been picked, and have the quote window's vm pick up that message and set the customer on the quote. Like I mentioned in my first post, this obfuscates the code, and wastes a lot of time searching around to find where the message is picked up, as well as having the issue of messages being picked up by the wrong code, or a profusion of very specialist messages.

My view is that given that the quote window knows exactly who needs to know about the picked customer, and has immediate access to the vm, I don't see what's wrong with pushing the customer down to the vm from the view. I'm not adding any testable logic to the view, it's very simple code. As I said, even the null check could be removed it you wanted.

Furthermore, if you use a message, and want to test the vm, you have to have some way of simulating the message. That probably means making the vm's ProcessMessage method public, and accessing it in the unit test. This feels really wrong, this method is private, and should stay private. The method I showed above doesn't have this problem, as you can just create a Customer entity in the unit test and pass it into the vm's SetCustomer() method.

What would you think to this scenario? Would you still be happy to have the code in the view, or would you feel a message is more appropriate here? If the latter, please can you explain what benefit a message has, as I really can't see any, but can see a few significant disadvantages.

Thanks again.