Is using getters in XAML view-models a bad thing?

Question

I've recently had an argument with a colleague about using getters (without setters) in a view-model classes used by XAML.

Example:

public string FullName
{
   get
   {
      return $"{FirstName} {LastName}";
   }
}
//call OnPropertyChanged("FullName") when setting FirstName or LastName

His argument was that you should only ever create get/set pairs which use a private field (with the setter raising the PropertyChanged event if the value has indeed been changed).

Example:

private string _fullName;
public string FullName
{
   get
   {
      return _fullName;
   }
   set
   {
      if (value != _fullName)
      {
         _fullName = value;
         OnPropertyChanged("FullName");
      }
   }
}
//Set FullName when setting FirstName or LastName

I, on the other hand, think it's fine to use getters (which calculate the output on the fly), provided they aren't updated all the time via too many PropertyChanged calls.

I don't think my colleagues approach is bad - I just think it's way more busy work which might simply be not needed. However he was sure my approach is bad and should be avoided at all cost.

As a counter example I pointed out that the MVVM base class from DevExpress has RaisePropertyChanged and RaisePropertiesChanged (for refreshing multiple properties) methods ready for use - both of which wouldn't be needed if all the properties were get/set pairs (the DevExpress base class also exposes a SetProperty method for use in setters, which also includes a PropertyChanged call). His argument was that "well, people are stubborn, keep doing it wrong, so DevExpress simply added those methods to make sure people are happy" which I found... odd, to say the least.

So what's the take here? Is it bad design to use getters in view-models designed to be used by XAML?

EDIT: I forgot to mention... my colleague also made the claim that using get/set pairs will always be faster. That does seem to be true as far as updating the UI is concerned, but he also used this as an argument that getters shouldn't ever be used. The whole argument started when I had a bit of performance issues when populating a data grid with more (i.e. 12k) rows where my VM used quite a few getters...

Answer 1

Your coworker is incorrect.

When working with UI logic in MVVM, it's common to have properties for binding that are derived from other data.

In these cases, you don't want to create another private member - that just adds a point of failure with no benefit.

Side note:

If you're using C# 6 then instead of hardcoded strings you can do

OnPropertyChanged(nameof(FullName));

This way, you'll get a compiler error if you change the name of the FullName property but forget to update corresponding calls to OnPropertyChanged.

Answer 2

Getters like your colleague expects, are a code smell. They're only there for language niggles anyway. What they're doing is exposing a private variable. To be honest, you might as well just access the private variable directly if that's all you're using getters for and cut out a layer of middleman.

Originally OO languages supported the use of accessor methods (eg getter and setters) as they intention was that they would perform validation and other logic on every get and set. It also hid the internal implementation details from the user of the class - so you could expose FullName, and then if you later changed the internals to store first and last names, you could update the getter without changing the clients who use your class. (and to expand further, when you extend your class to handle middle initial, that too can be done without having to update every caller)

Now over time, code generators like Devexpress and VS added shortcuts to help, by letting you declare a variable private and automatically type out the boilerplate to write the accessors. Unfortunately, some people think that because this happens, the boilerplated accessor is what you should use. Its nuts that people don't think for themselves and have descended into a cargo-cult attitude to coding.

There are articles on the web that describe how you should be designing classes, that say if all you're doing is exposing a variable as a getter, then you haven't actually designed anything.

And lastly, if you do store the same data twice (fullname as well as first and last names) then you are starting to get in a mess, as you will always have to do plenty of updating whenever any of the 3 change.

Answer 3

The colleague is wrong. Mutable state should be kept at minimum/normalized and what can be readonly/calculated should be calculated i.e. fewer setters is better. The only good excuse for denormalizing mutable state is performance tuning.

As for INotifyPropertyChanged notificarions of readonly properties - do NOT raise them manually from trigger property setter, this is maintainability killer.

Instead use this awesome library, Calculated Properties: https://github.com/StephenCleary/CalculatedProperties/blob/master/README.md

I never had to raise PropertyChanged manually and never had bugs that something is not refreshed etc. since I discovered this library, it is invaluable