0

I have a WPF window that allows the user to edit the Date of Birth of an employee, in this window is also a TextBox to display the age from the DoB entered.

The DoB textbox is bound to my EmployeeModel Dob property

The Age textbox is bound to my EmployeeModel Age property and cannot be set, and the get using the Dob value.

The problem I'm facing is when the user changes the DoB, the Age textbox does not get updated.

I have found that if I implement INotifyPropertyChanged in my EmployeeModel, and add OnPropertyChanged("Age") in the Dob property, then the Age value gets updated. But this goes against what I have been advised that my Models should be business logic only and should not implement INotifyPropertyChanged.

How should I set this up so Age updates when DoB is changed without modifying the Model?


EmployeeModel

class EmployeeModel
{
    public DateTime? Dob { get => _dob; set => _dob = value; }
    public int Age
    {
        get
        {
            if (_dob == null)
                return 0;

            DateTime now = DateTime.Today;
            int age = now.Year - _dob.Value.Year;
            if (now < _dob.Value.AddYears(age))
                age--;

            return age;
        }
    }
}

EmployeeViewModel

class EmployeeViewModel : INotifyPropertyChanged
{
    private EmployeeModel _employee;

    public EmployeeModel Employee
    {
        get => _employee;
        set
        {
            if (_employee != value)
            {
                _employee = value;
                OnPropertyChanged();
            }
        }
    }
    public event PropertyChangedEventHandler PropertyChanged;

    protected void OnPropertyChanged([System.Runtime.CompilerServices.CallerMemberName]string caller = null)
    {
        if (PropertyChanged != null)
            PropertyChanged(this, new PropertyChangedEventArgs(caller));
    }

View

<Label Content="DOB" />
<DatePicker SelectedDate="{Binding Employee.Dob, TargetNullValue=''}" />
<Label Content="Age" />
<TextBox Text="{Binding Employee.Age, Mode=OneWay}" />
Lajos Arpad
  • 64,414
  • 37
  • 100
  • 175
smally
  • 453
  • 1
  • 4
  • 14
  • 1
    When the employee birthdate changes, just raise property changed on the age property. – Kevin Cook Jun 19 '19 at 11:35
  • "Models should be business logic only and should not implement INotifyPropertyChanged", this is opinion based. Look at [this](https://stackoverflow.com/questions/772214/in-mvvm-should-the-viewmodel-or-model-implement-inotifypropertychanged/4574762). The accepted answer says you shouldn't, but the answer with the most votes says you can. – Hyarus Jun 19 '19 at 11:50
  • The binding mode of the `TextBox` must be `TwoWay` in order to send the user input back to the `Age` property. Just remove the mode from the binding since it is `TwoWay` by default for the `TextBox` class. Otherwise if you just want to display the age to the user, then better use a `TextBlock`, since it is cheaper resource and performance wise (but at least set `IsReadOnly=True`). – BionicCode Jun 20 '19 at 11:24
  • Also if you don't like to bind directly to your model, which is right, then why do you expose it as a public property in your `EmployeeViewModel` class? The property must be private. You have to duplicate the properties of the model on the view model and delegate property calls to the private model like `Age { get { return this._employee.age; } set { this._employee.Age = value; } }`. – BionicCode Jun 20 '19 at 11:33
  • @Hyarus Since in programming you always _can_ do anything (as long your compiler let's it go), the _should_ and _shouldn't_ are important. It's referred to as best practice. You can't introduce MVVM to your application and the bind to your models. This is not MVVM, since it encourages strict separation of the model from the view model for some good reason (e.g. to improve testablity and extensibility). You can implement it in your model but shouldn't ever `bind` to it. – BionicCode Jun 20 '19 at 11:49
  • @Hyarus The point is `INotifyPropertyChanged` is a generic event that is introduced for one purpose only in mind: data binding (framework internal). The framework doesn't care what the purpose of a property is, since it just 'transports' the data from binding source to binding target. As a programmer or user of some API your very interested in which particular event was raised or which particular property changed. That's why you should introduce specific events (e.g. AgeChanged). – BionicCode Jun 20 '19 at 11:49
  • @Hyarus `INotifyPropertyChanged` would require you to filter the proper property by name in order to identify the property that actually changed, which is not very convenient. Therefore the dedicated changed event everywhere when possible. Maybe you even add an explicitly named event to your view model (that duplicates the PropertyChanged event) when you depend on it (and in this case to improve your code). That's why when mentioning INotifyPropertyChanged everybody thinks of binding in first place. – BionicCode Jun 20 '19 at 11:58

2 Answers2

1

If you don't want to implement INotifyPropertyChanged in EmployeeModel you shouldn't bind to a property of an EmployeeModel but to a property of the view model that wraps the property in EmployeeModel, e.g.:

class EmployeeViewModel : INotifyPropertyChanged
{
    private EmployeeModel _employee;

    public DateTime? Dob
    {
        get { return _employee.Dob; }
        set { _employee.Dob = value; OnPropertyChanged(); OnPropertyChanged(nameof(Age)); }
    }

    public int Age
    {
        get { return _employee.Age; }
    }

    public event PropertyChangedEventHandler PropertyChanged;

    protected void OnPropertyChanged([System.Runtime.CompilerServices.CallerMemberName]string caller = null)
    {
        if (PropertyChanged != null)
            PropertyChanged(this, new PropertyChangedEventArgs(caller));
    }
}

XAML:

<Label Content="DOB" />
<DatePicker SelectedDate="{Binding Dob, TargetNullValue=''}" />
<Label Content="Age" />
<TextBox Text="{Binding Age, Mode=OneWay}" />
mm8
  • 163,881
  • 10
  • 57
  • 88
  • Along with this, we need to add UpdateSourceTrigger=PropertyChanged to DataPicker – Krish Jun 27 '19 at 13:39
  • Is doing your suggested method a requirement for all the fields. My view actually has over 50 editable fields. Age and FullName are the only ones that requires information from fields not related to their own. (FullName is slightly different as it is a label, and fullname does not exist in my EmployeeModel) – smally Jun 30 '19 at 20:00
  • @smally: Yes, of course. You need to raise the `PropertyChanged` event for all data-bound properties that you want to refresh. – mm8 Jul 01 '19 at 11:07
0

I am also using your approach to calculate the age and it works properly.

I have a Patient model which is defined in a separate class

Here is the ViewModel:

class PatientRecordDetailsViewModel : INotifyPropertyChanged
{
    public DateTime PatientDateOfBirth
    {
        get => m_patientDateofbirth;
        set
        {
            m_patientDateofbirth = value;
            OnPropertyChange("PatientDateOfBirth");
            OnPropertyChange(nameof(PatientAge));
        }
    }

    public int PatientAge => CalculateAge();

    private int CalculateAge()
    {
        if (m_patientDateofbirth == null)
        {
            return 0;
        }
        else
        {
            int CurrentYear = DateTime.Now.Year;
            int BirthYear = m_patientDateofbirth.Year;
            m_patientAge = CurrentYear - BirthYear;
            return m_patientAge;
        }           
    }

    public event PropertyChangedEventHandler PropertyChanged;
     private void OnPropertyChange(string propertyName)
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
    }
}

Here is the Xaml

<DatePicker Name="Date" Grid.Column="0" SelectedDate ="{Binding Path = PatientDateOfBirth, UpdateSourceTrigger=PropertyChanged}" Style="{StaticResource DatePickerStyle}" VerticalAlignment="Center" Height="30" Padding="3,0,5,0"/>
<TextBox  Name="Age" Grid.Column="1" Text="{Binding Path = PatientAge, Mode=OneWay}" Style="{StaticResource TextBoxStyle}"/>
Senali
  • 25
  • 4