Why does `PropertyChanged?.Invoke()` only work in the concerning object but not from the ViewModel

Question

I have a JobView which contains a ItemsControl. It displays a DataTemplate for each "job" in Jobs. A job can be started/stopped by clicking the buttons in the DataTemplate. The DataTemplate also contains the Status of a job (started/stopped) but this is not updated when in trigger the PropertyChanged?.Invoke(...) event from the JobViewModel.

When i move the PropertyChanged?.Invoke(...) to the (abstract) RJob class (for a specific variable in that class), it works fine thought this does not feel nice to me. I don't want to add "view logic" to my classes to update a view. I want to keep this logic in the ViewModels.

I also want to avoid that i need to do a PropertyChanged?.Invoke(...) for each variable in a class (RJob in this example) that i show in a view. I prefer to do something like PropertyChanged?.Invoke(job) so i automatically updates all showed variables of the concerning job.

I have 2 questions about this:

What is de convinient place to trigger the PropertyChanged?.Invoke(...)?
Is it possible to trigger the PropertyChanged?.Invoke(...) on object level instead of on property level so it updates all binded properties without creating a PropertyChanged?.Invoke(...) for each individual property?

Note that i cleaned up the code a bit for this question.

My JobView.xaml:

<UserControl x:Class="ECRO.MVVM.View.JobView"
             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008" 
             xmlns:viewModel="clr-namespace:ECRO.MVVM.ViewModel"
             mc:Ignorable="d" d:DesignWidth="800" Height="780">

    <UserControl.DataContext>
        <viewModel:JobViewModel/>
    </UserControl.DataContext>

    <UserControl.Resources>
            <DataTemplate x:Key="jobTemplate">
                <Grid Height="90" Width="760">
                    <Grid.ColumnDefinitions>
                        <ColumnDefinition Width="677*"/>
                        <ColumnDefinition Width="123*"/>
                    </Grid.ColumnDefinitions>
                    <Border CornerRadius="10"
                    Background="#353340"
                    Height="80"
                    Margin="10,0,10,0"
                    VerticalAlignment="Center" Grid.ColumnSpan="2">
                        <Grid>
                            ...

                        <StackPanel Orientation="Vertical"
                                    Margin="5 2 0 0">
                            <Label Content="Naam" FontWeight="Bold" Style="{StaticResource DefaultLabel}" Margin="0 -5 0 -5"/>
                            <Label Content="Status" FontWeight="Bold" Style="{StaticResource DefaultLabel}" Margin="0 -5 0 -5"/>
                        </StackPanel>
                        <StackPanel Grid.Column="1" Margin="2">
                            <Label Content="{Binding Name}" Style="{StaticResource DefaultLabel}" Margin="0 -5 0 -5"/>
                            <Label Content="{Binding Status}"  Style="{StaticResource DefaultLabel}" Margin="0 -5 0 -5"/>
                        </StackPanel>


                            <Border Grid.RowSpan="2"
                                    Grid.Column="2"
                                    Grid.Row="0"
                                    HorizontalAlignment="Right"
                                    Margin="0 0 20 0">
                                <StackPanel Orientation="Horizontal">
                                    <Button Content="Start" 
                                        Style="{StaticResource DarkButton}" 
                                        Command="{Binding DataContext.StartJobCmd, RelativeSource={RelativeSource AncestorType=ItemsControl}}"
                                        CommandParameter="{Binding}"/>
                                    <Button Content="Stop" 
                                        Style="{StaticResource DarkButton}" 
                                        Command="{Binding DataContext.StopJobCmd, RelativeSource={RelativeSource AncestorType=ItemsControl}}"
                                        CommandParameter="{Binding}"/>
                                   ...
                                </StackPanel>
                            </Border>
                        </Grid>
                    </Border>
                </Grid>
            </DataTemplate>
    </UserControl.Resources>

    <StackPanel>
        <TextBlock Text="Jobs" Style="{StaticResource HeaderTextBlock}"/>
        <ScrollViewer Width="780" Height="700">
            <ItemsControl ItemsSource="{Binding Jobs}" 
                          ItemTemplate="{StaticResource jobTemplate}"/>
        </ScrollViewer>
    </StackPanel>
</UserControl>

Not working JobViewModel with RJob:

My JobViewModel

using ECRO.Class;
using ECRO.Core;
using ECRO.RJobs;
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.ComponentModel;
using System.Diagnostics;

namespace ECRO.MVVM.ViewModel
{
    class JobViewModel : INotifyPropertyChanged
    {
        private ObservableCollection<RJob> _jobs;
        public ObservableCollection<RJob> Jobs { get => _jobs; set { _jobs = value; } }

        public event PropertyChangedEventHandler? PropertyChanged;

        public RelayCommand StartJobCmd { get; set; }
        public RelayCommand StopJobCmd { get; set; }

        ...

        public JobViewModel()
        {
            _jobs = Engine.Instance.RJobs;

            StartJobCmd = new RelayCommand(o => { 
                RJob job = (RJob)o;
                job.Start();
                PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(job)));
            });
            StopJobCmd = new RelayCommand(o => {
                RJob job = (RJob)o;
                job.Stop();
                PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(job)));
            });
            ...
        }
    }
}

My abstract RJob which is inherrited by all jobs:

...

namespace ECRO.Class
{
    public abstract class RJob
    {
        private JobStatus jobStatus;
        public JobStatus Status { get => this.jobStatus; }

        ...

        public JobStatus Start()
        {
            if (active && jobStatus != JobStatus.Running)
            {
                try {
                    runTask();
                    jobStatus = JobStatus.Running;
                } catch (Exception e) {
                    this.jobStatus = JobStatus.Failed;
                    Log.Error($"Some error occurred in job \"{Name}\", check the job log for more details.");
                    Log.Error(e, this);
                }
            }
            SaveConfiguration();
            return jobStatus;
        }

        public JobStatus Stop(bool saveConfig = true)
        {
            if (jobStatus == JobStatus.Running)
            {
                jobExcecuteTimer.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan);
                jobStatus = JobStatus.Stopped;
            }
            if (saveConfig)
            {
                SaveConfiguration();
            }
            return jobStatus;
        }

        ...
    }
}

Working JobViewModel & RJob:

My JobViewModel

using ECRO.Class;
using ECRO.Core;
using ECRO.RJobs;
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.ComponentModel;
using System.Diagnostics;

namespace ECRO.MVVM.ViewModel
{
    class JobViewModel 
    {
        private ObservableCollection<RJob> _jobs;
        public ObservableCollection<RJob> Jobs { get => _jobs; set { _jobs = value; } }

        public RelayCommand StartJobCmd { get; set; }
        public RelayCommand StopJobCmd { get; set; }

        ...

        public JobViewModel()
        {
            _jobs = Engine.Instance.RJobs;

            StartJobCmd = new RelayCommand(o => { 
                ((RJob)o).Start();
            });
            StopJobCmd = new RelayCommand(o => {
                ((RJob)o).Stop();
            });
            ...
        }
    }
}

My abstract RJob which is inherrited by all jobs:

...

namespace ECRO.Class
{
    public abstract class RJob : INotifyPropertyChanged
    {
        private JobStatus jobStatus;
        public JobStatus Status { get => this.jobStatus; }
        
        ...

        public JobStatus Start()
        {
            if (active && jobStatus != JobStatus.Running)
            {
                try {
                    runTask();
                    jobStatus = JobStatus.Running;
                    PropertyChanged?.Invoke(this, new PropertyChangedEventArgs("Status"));
                } catch (Exception e) {
                    this.jobStatus = JobStatus.Failed;
                    Log.Error($"Some error occurred in job \"{Name}\", check the job log for more details.");
                    Log.Error(e, this);
                }
            }
            SaveConfiguration();
            return jobStatus;
        }

        public JobStatus Stop(bool saveConfig = true)
        {
            if (jobStatus == JobStatus.Running)
            {
                jobExcecuteTimer.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan);
                jobStatus = JobStatus.Stopped;
                PropertyChanged?.Invoke(this, new PropertyChangedEventArgs("Status"));   
            }
            if (saveConfig)
            {
                SaveConfiguration();
            }
            return jobStatus;
        }
        
        ...
    }
}

RJob must implement INotifyPropertyChanged and fire the PropertyChanged event for every property that is shown in the view. That is the standard implementation. — Clemens, Aug 17 '23 at 08:24
Ok, that would mean that i must add lots and lots of `PropertyChanged` events in the whole class. Is it possible to make a kind of general implementation in a class which fires the event if one of its properties is updated? It still feels dirty to fire these events in "logic/core" classes to update views. — CodeNinja, Aug 17 '23 at 09:00
You should typically not bind to your model, or "logic/core" classes at all. That is what the viewModel is for. So you might consider renaming JobViewModel to Job**s**ViewModel, and create a proper JobViewModel that owns a Job, and takes care of all the UI stuff, delegating any logic to the actual job object. — JonasH, Aug 17 '23 at 09:08
You may save the boilerplate code by using [ObservableProperty](https://learn.microsoft.com/en-us/dotnet/communitytoolkit/mvvm/generators/observableproperty) with [ObservableObject](https://learn.microsoft.com/en-us/dotnet/communitytoolkit/mvvm/observableobject). — Clemens, Aug 17 '23 at 09:12

score 1 · Answer 1 · answered Aug 17 '23 at 08:52

What is de convinient place to trigger the PropertyChanged?.Invoke(...)

My preference, is to have a OnPropertyChanged method, and a SetField method:

public event PropertyChangedEventHandler PropertyChanged;
protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null) => PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
protected bool SetField<T>(ref T field, T value, [CallerMemberName] string propertyName = null)
{
    if (EqualityComparer<T>.Default.Equals(field, value)) return false;
    field = value;
    OnPropertyChanged(propertyName);
    return true;
}

// Used like
public JobStatus Status
{
    get => status;
    set => SetField(ref status, value);
}

This tend to reduce code duplication when writing properties. And if you just set the property rather than the field you do not have to worry about calling PropertyChanged directly.

Is it possible to trigger the PropertyChanged?.Invoke(...) on object level instead of on property level so it updates all binded properties without creating a PropertyChanged?.Invoke(...) for each individual property

You can invoke PropertyChanged with null or string.Empty to mark all properties as changed. See PropertyChanged for all Properties in ViewModel?. But it is not something I commonly use. I find that the pattern above is compact and easy enough that there is little benefit just to reduce code.

You can also do something like

public RJob MyJob
{
    get => myJob;
    set => SetField(ref myJob, value);
}

And setting that will update everything bound to a property of MyJob. But this does not seem relevant in your case, since you have a ObservableCollection<RJob>, and not a property.

Thanks for your usefull answer. I see that you also call the `OnPropertyChanged` in the class/object and not in the `ViewModel`. Seems to be common but it still feels like the wrong place to me as its view related and i prefer to split view and logic pretty hard. I will also check/test the suggestion in question you linked to. I do like the `SetField` suggestion (when the event is fired in the object/class). Unfortunately i cannot use it in this situation as most of the `Job` properties are not public setable. — CodeNinja, Aug 17 '23 at 09:37
@CodeNinja then make the property setters private? You may need to configure the binding to work with read-only properties, but it should work. And I *do* usually put OnPropertyChanged on the viewModel. But I'm more pragmatic than dogmatic, so exceptions certainly do occur. — JonasH, Aug 17 '23 at 09:50

Why does `PropertyChanged?.Invoke()` only work in the concerning object but not from the ViewModel

1 Answers1