1

Scenario.

  • ViewModel has a property State.
  • View has a ComboBox that allows to change value of State.
  • ViewModel needs to run some validation (call bool ValidateState(State value) method) before setting value of State property.

Question.

How would you implement it?

My concerns.

  • I want my solution to be generic so I could use it in different similar scenarios. For example in scenario where ValidateState() is an async method.
  • I do not want change value of State in ViewModel unless I 100% sure it's valid.
  • If new value of State is not accepted (is not valid) I want ComboBox to keep having the old value as selected.

Disclaimer. I know there are multiple ways of implementing it. Also I'm not a MVVM novice. I would like to discuss which solution you think better suits this scenario and possibly discuss pros and cons of it.

  • Why do you want validate before setting it? That is not something that is normally done. – Euphoric Nov 21 '17 at 13:28
  • @Euphoric Let me, maybe, provide you with a more specific scenario. Logged in user has some set of permissions. Depending on a particular permission application before setting the value of State property should check if some other property of ViewModel has a value. If not then the app should prompt user for a confimration whether it's OK to change state even though that other property does not have a value. Hopefully it gives you more context. If you think this information is important I can add it to the main question desription. – Pavels Ahmadulins Nov 21 '17 at 13:53
  • 1
    This might be question for UX.StackExchange, but wouldn't it be better if you had "Current value" field that is read-only and "Set to" field with button to set it? It would simplify the operation and would it make clearer that setting the value is not so straightforward. Also, I remember WPF checkbox doesn't like if you have this kind of "validation" under it. Eg. if you don't set the value that it sends. – Euphoric Nov 21 '17 at 14:08
  • I've considered this option. But do you think it's OK to flood ViewModel with lots of miscelaneous properties whose sole purpose it to make ViewModel being easier to use by View? I thought that ViewModel shoul only expose necessary minimum of API to View. Otherwise teoretically each significant upgrade of view layer will require updating ViewModel layer because what was convenient today may not be convenient tomorrow. This approach looks like a code smell (tight coupling perhaps). – Pavels Ahmadulins Nov 21 '17 at 14:28
  • 1
    I wouldn't consider it tight coupling. I would see it more as ViewModel clearly communicating that some property's setter might cause additional actions and that it might not be trully set if value is sent to it. This is Principle of Least Surprise in action. – Euphoric Nov 21 '17 at 15:01
2

You could implement INotifyPropertyChanged interface and use the setters to perform the validations. After validation succeeds, you set the private field and notify the changed property for the UI to be updated according to ViewModel's State.

Here goes some code as example.

class YourViewModel : INotifyPropertyChanged {
    private State _state;

    public event PropertyChangedEventHandler PropertyChanged;

    public State State {
        public get { return _state; }
        public set {
            ValidateState(value);
            //If async operation is required, you could set
            //this ViewModel state to "Validating" or something like that
        }
    }

    //ValidateState
    void ValidateState(State newState) {
        //Perform validations
        if (validEnough) {
            //Change the actual state, and also the UI state
            _state = newState;

            //Tells ui to update its view
            PropertyChanged(this, new PropertyChangedEventArgs("State"));
        }
    }

}

//Example of unit test
[TestMethod]
public void SetState_HasNoPermissions_DoesNotChangeState() {
    //... setup resources for the test
    yourViewModel.State = State.FirstStep;

    Assert.NotEquals(yourViewModel.State, State.FirstStep);
}

Update to address some comments

... I personally think ViewModel should only contain logic, but should not know anything about View

That's correct. The important stuff of your screen is the State, not the combobox (which is not known by the viewModel). I can't see how my example fails this.

Examples: ... it's not OK to generate readable text for View like error meessages ...

Who said it is no ok to do that? Do you see any problem in having a viewModel loading localized messages at runtime, for example? If it's OK or not will depend on your application needs.

So the thing I'm trying to say is I do not agree with this: here your viewModel IS the user interface.

What I tried to say is that graphics and visual stuff are placed ONLY in the View, and the state and behaviour are placed in the ViewModel. So, therefore, you GUIs are represented partly by the View, and partly by the ViewModel. That's why I said that in the example, the viewModel is your user interface, since i'm not considering details such as Combobox or GridView or whatever; I'm only interested in the behaviour of the screen, and this behavior should be entirely implemented in your viewModel, if you chose to follow MVVM pattern.

But, you don't need to believe me, please just read Presentation Model (another name for the same pattern) article and the other source mentioned below.

Also it's not view that requires validation - bussinss logic ruquires it. And therefore it should implemented in the ViewModel layer rather then in View layer.

What I meant is that there might be a requirement like: "When user changes the value of the state, there must be performed a validation on it, and the view must update this value only if validation succeeds."

Because of that, you want to implement some behavior on you GUI: the user changes a combobox' value, and the validation occurs; if validation fails, combobox's value stays the same; otherwise a new value is set in the combobox.

The above statement is implemented by my example, following ModelView-ViewModel design pattern. Actually, in the code example I put ValidateState in the ViewModel, but the idea is that the viewModel triggers the validation logic that should be implemented somewhere else; the code is to serve just as an example to aid you in your development scenario. But, if anything above seems like bad practice after all to you, this is just a suggestion anyways :)

Programming best practices are same for everything. If you use MVVM architecture pattern it doesn't mean you should be allowed introducing code smells.

I agree and I personally don't see the code smell here.

One last thing. If your viewModel IS the user interface then how is it different from code behind file? ... What is the actual benefit of separating ViewModel from View if ViewModel "serves" View?

Let me answer that by quoting Martin Fowler in his Presentation Model article:

Presentation Model: Represent the state and behavior of the presentation independently of the GUI controls used in the interface

...And by quoting MVVM article from Msdn:

It provides separation of concerns. ... A clean separation between application logic and the UI will make an application easier to test, maintain, and evolve. It improves code re-use opportunities and enables the developer-designer workflow.

About the code-behind file: this is just the C# part of your XAML. All code you put there should address only presentation concerns; ideally, behavior and state of your GUI should be implemented in the ViewModel classes. At least this is what is described as the pattern, by the sources below. I suggest you to fully read and understand below articles.

Sources:

https://martinfowler.com/eaaDev/PresentationModel.html

https://msdn.microsoft.com/en-us/library/hh848246.aspx

  • 1
    Exactly what I had in mind. That is, after all, one of the primary reason for having a setter. – jleach Nov 22 '17 at 8:55
  • 1
    If we talk about programming in general - I thought that good practice is to keep properties lightweight and do not put any complicated logic inside. Validation can be quite complex if we need to fetch latest updates from user permission provider, for example. Also how exactly will you bind this property to ComboBox? Are you sure that if validation fails you will not end up having one value in ComboBox and different value in ViewModel? – Pavels Ahmadulins Nov 22 '17 at 9:12
  • Validation can be quite complex - you could then make it async, setting the state to "validating", and after validation completes, you either go to a new state or to the old one; this is easily implemented by my suggestion. how exactly will you bind this property to ComboBox? - this is basic databinding, you set the data source of the view as you viewModel object and then create a bidirectional databinding of the combobox to the State property of the viewModel; this is simply the view subscribing to the INotify interface – Emerson Cardoso Nov 22 '17 at 9:58
  • 1
    I've just tried to see what happens when validation fails and property value is not updated in ViewModel. Actually there are three parts that play role - visual (what user sees in a screen), value of ComboBox.SelectedItem, and ViewModel.State. So what happened is values of ComboBox.SelectedItem and ViewModel.State stayed the same (correct) but visually ComboBox has switched to a new selected value anyway (wrong). – Pavels Ahmadulins Nov 22 '17 at 10:31
  • 1
    There should also be an error provider which shows any validation errors to the user, thus indicating that something went wrong. And the value of the error provider must be set/unset during validation. The (additionally) required interface is INotifyDataErrorInfo. – Bernhard Hiller Nov 22 '17 at 11:00
1

Here is my version of implementation:

View:

<CheckBox 
    Style="{StaticResource CheckBoxStyle}"
    Content="Accept new value" 
    IsChecked="{Binding Path=IsValid, Mode=TwoWay}"
/>

<ComboBox
    Style="{StaticResource ComboBoxStyle}"
    ItemsSource="{Binding Path=States, Mode=OneTime}"
    SelectedItem="{Binding Path=State, Mode=OneWay}"
    SelectionChanged="ComboBox_SelectionChanged"
/>

View code-behind:

private void ComboBox_SelectionChanged(object sender, SelectionChangedEventArgs e)
{
    var comboBox = (ComboBox)sender;
    var isSuccess = _viewModel.StateSetTry((string)comboBox.SelectedItem);

    if (!isSuccess)
    {
        comboBox.GetBindingExpression(ComboBox.SelectedItemProperty).UpdateTarget();
    }
}

ViewModel:

public class ViewModel : BindableBase
{
    private string _state = "State one";
    private bool _isValid = true;

    public ImmutableArray<string> States { get; } = 
        new [] { "State one", "State two", "State three" }.ToImmutableArray();

    public string State
    {
        get => _state;
        private set => SetProperty(ref _state, value);
    }

    public bool IsValid
    {
        get => _isValid;
        set => SetProperty(ref _isValid, value);
    }

    public bool StateSetTry(string value)
    {
        if (IsValid)
        {
            State = value;
        }

        return IsValid;
    }
}

Pros:

  • Simple ViewModel public interface (there are no additional "helper" properties)
  • Property getters/setters remain simple (follows programming best practices)
  • Easy to convert to async/await method (just make StateSetTry retrun Task<bool>)

Cons:

  • Not conventional MVVM approach
  • Requires code in code behind (it is possible to move it to a separate Behaviur class but it will probably require using Reflection, similar to CallMethodAction)

I think that the fact that View cannot call (or bind in some way) public bool StateSetTry(string value) method of ViewModel from XAML code is a problem of View layer and therefore it should be solved/handled by View layer.

Also I think that ViewModel layer should not generate human readable text on error because of the way ViewModel can be re-used. With Silverlight and Windows Phone technologies becoming obsolete the only case when I ever had to re-use ViewModel in a commercial application was white labeling. Two different application had the same logic but different UI. In this case these two different application could aim different audiences - one aims more technically savyy users and the other user not familiar with technologies. Therefore in some cases the same general or validation error can be handled differently - one can display a message with detailed description, and the other can try to solve the problem automatically and hide details from users and therefore will not display any error message. Besides View can also be a console application that returns integer error codes if there is bussines requirement for that.

Your Answer

By clicking "Post Your Answer", you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.