MVVM Light ViewModelBase vs Encapsulation

Question

I've been using MVVM Light for a while now - it's extremely useful and almost always the first library I add to a new project!

I'm wondering what the implications would be of developing a class that implements INotifyPropertyChanged to encapsulate a bindable property (example below).

public class BindableProperty<T> : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;


    private T mValue;
    public T Value
    {
        get { return mValue; }
        set
        {
            if (!EqualityComparer<T>.Default.Equals(mValue, value))
            {
                mValue = value;
                if (PropertyChanged != null)
                {
                    PropertyChanged(this, new PropertyChangedEventArgs("Value"));
                }
            }
        }
    }


    public BindableProperty(T default_value)
    {
        mValue = default_value;
    }
}

With this class, I have to change my Xaml, but I believe my ViewModel might be more readable (below) - especially when the number of properties grows.

<TextBox Text="{Binding FirstName.Value, UpdateSourceTrigger=PropertyChanged}"/>

public class MainVM
{
    public BindableProperty<string> FirstName { get; private set; }
    public BindableProperty<string> LastName { get; private set; }

    public MainVM()
    {
        FirstName = new BindableProperty<string>("");
        LastName = new BindableProperty<string>("");
    }
}

I know MVVM Light is designed to be extremely flexible, lightweight, and provide complete control (which it does very well). I can of course combine implementations and use the BindableProperty class above for some properties and more explicit ViewModelBase code for other properties in more complex situations.

Am I missing something obvious? What are some of the trade offs for this design that I might not be aware of (e.g. implications of changing xaml binding, data validation...)?

I may have missed what you are trying to get across, but I think what you should be doing is implementing a `dependency property`. — Hugh Jones, Mar 08 '16 at 15:22
Have a look at the implementation of the [BindableBase](https://github.com/PrismLibrary/Prism/blob/master/Source/Prism/Mvvm/BindableBase.cs) class in the PRISM framework and it's [usage](http://stackoverflow.com/questions/28844518/bindablebase-vs-inotifychanged). One adventage is, that you bind to the property itself, not to `property.Value`. — Batuu, Mar 08 '16 at 15:25
@Batuu - BindableBase looks very similar to mvvm-light [ViewModelBase](http://www.mvvmlight.net/help/SL5/html/e1c28204-2508-66c2-f984-25ba65c04a30.htm). Maybe it's not worth the effort, but using this boilerplate code typically puts 5 lines per property in my ViewModel to make it look clean, opposed to 1 line using the class above (granted 1 more in constructor). — sfm, Mar 08 '16 at 15:33
@HughJones: Dependency properties are a view concern and pattern, they do not belong to a ViewModel — Tseng, Mar 09 '16 at 08:16
@tseng - good point. I am still not that clear on what the OP wants to achieve, though. — Hugh Jones, Mar 09 '16 at 09:17
@HughJones: He wants to avoid implementing getter/setter for every INPC property, where he validates if a value changed, assign it and call OnPropertyChanged and use autoproperties instead — Tseng, Mar 09 '16 at 09:45

score 0 · Answer 1 · answered Mar 09 '16 at 08:40

There is no extra value in encapsulating a property. This may work for very simple scenarios, but as soon as your ViewModel becomes more complex, you'll end up wiring your encapsulated classes in odd ways.

This approach won't work very well with validation neither it will if you have properties that depend on each other, i.e. FirstName, LastName and FullName where FullName is just a public string FullName { get { return FirstName+" "+LastName; } }.

Same applies for validation (with IDataErrorInfo). With the base class your code looks like

public string FirstName 
{
    get { return firstName; }
    set
    {
        if(string.IsNullOrEmpty(value)) 
        {
             // where errors is a Dictionary<string, string>
             errors.Add(nameof(FirstName), "First name can't be empty.");
             return;
        }

        if(value.Length <2) 
        {
             errors.Add(nameof(FirstName), "First name must be at least 2 characters long.");
             return
        }

        Set(ref firstName, value);
        errors.Remove(nameof(FirstName));
    }
}

This will be a pain to implement in encapsulated properties

I agree for more complex scenarios, you need the flexibility to write code shown above. But for very simple cases, I feel like this code makes my VMs hard to scan and understand their purpose. For me, the simplest of properties ends up being 7 lines (backing variable:1 line, property: 4 lines, spacing: 2 lines). I think this makes it hard to digest and quickly understand the function of the VM. If I use something like the class from the original question - I have 1 line per property at the top of my VM. — sfm, Mar 09 '16 at 14:39

MVVM Light ViewModelBase vs Encapsulation

1 Answers1