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.
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:
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.
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:
UpdateDateTimeOffsetProperty, and so on. This feels like it would be mitigating the symptom instead of addressing the underlying issue.
Problem: Method Archetypes
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
This mostly works pretty well. Here’s the simplified
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
where E: Enum. Instead, we need to give it a different name:
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.