When I went to write the code for our next view model (VocabularyBrowserViewModel), I ran into some problems with our original ViewModelBase. Let’s visit those problems and look at how they can be resolved.

Problem: Boilerplate

The original motivation for introducing the UpdateProperty() method overloads was to eliminate (or, at least, reduce) the amount of boilerplate needed to write properties for our view-models.

In practice, I found that almost all of my view model properties ended up looking like this:

public string Spelling
{
    get => _spelling;
    set
    {
        if (UpdateProperty(ref _spelling, value))
        {
            _store.Dispatch(new ModifySpellingMessage(_spelling));
        }
    }
}

Every property was testing the return value from UpdateProperty to see if the value changed, and then doing something if it had.

Clearly, there is substantial room for improvement here to reduce the amount of boilerplate needed for each property.

Problem: ViewModel<T>

The intent of the ViewModel<T> class was to provide a useful base class for when the view-model was going to wrap a single model object. Turns out that this isn’t that common a scenario.

Our first view-model class (AddVocabularyWordViewModel) wrapped just a single screen, but the second class needs to pull in more information. In addition to the properties on the screen, the view needs the word list itself.

It appears that this base class isn’t really providing any benefit, so let’s remove it for now. We can always bring it back later on if we find a need.

Problem: Type Restrictions

We originally had five different overloads of UpdateProperty, each for a different primitive type, but otherwise essentially identical.

This approach fell apart as soon as I reached our second view-model because it doesn’t support the variety of property types we need. For example, we need a collection of words that we can show to the user, and we also need to track the user’s current selection.

Adding more overloads of UpdateProperty feels like a mistake - and proved to be one too. I found the approach quickly fell apart as the number of types increased. I ended up with a situation where different overloads overlapped, causing compiler errors.

One solution would be to rename each overload to reflect the supported type: UpdateIntProperty, UpdateDateTimeOffsetProperty, and so on. This feels like it would be mitigating the symptom instead of addressing the underlying issue.

Problem: Method Archetypes

The UpdateProperty methods were a little confused - they were commands that modified the object, but they returned a value much like a query. For anyone familiar with method archetypes, this design should trigger a second look.

Regrettably, I ignored this code smell when doing the original implementation - and now we’re paying the price for that.

Finding the pit of success

To reduce the amount of boilerplate, we can move the if-changed check into the helper method, as long as we pass in a delegate for the thing that should happen when the value changes.

Combine this with careful use of generics, to collapse all the different overloads together, and we can write a single UpdateProperty method:

protected void UpdateProperty<T>(
    ref T member,
    T newValue,
    Action<T> whenChanged = null,
    [CallerMemberName] string property = null)
    where T : IEquatable<T>
{
    if (member?.Equals(newValue) == true)
    {
        return;
    }

    member = newValue;
    OnPropertyChanged(property);

    whenChanged?.Invoke(newValue);
}

This mostly works pretty well. Here’s the simplified Spelling property:

public string Spelling
{
    get => _spelling;
    set => UpdateProperty(
        ref _spelling,
        value,
        sp => _store.Dispatch(new ModifySpellingMessage(sp)));
}

Unfortunately, the generic constraint requiring IEquatable<T> prevents the new UpdateProperty method from being used with enum valued properties.

We could remove the constraint, permitting enumerations. But, this would be at the cost of allowing properties where equality isn’t well defined for the type. This could lead to subtle bugs - such as excessive UI flickering - and isn’t an attractive solution.

Regrettably, generic constraints don’t form a part of the method signature for the purposes of overload selection, so we can’t just declare an overload of UpdateProperty with where E: Enum. Instead, we need to give it a different name: UpdateEnumProperty.

Hopefully this modified version of ViewModelBase will serve us well.

With these problems addressed, we can look at our next view-model, for browsing a word list.

Prior post in this series:
First Light
Next post in this series:
Vocabulary Browser

Comments

blog comments powered by Disqus