OnPropertyChanged is called twice if we use the ViewModelToModel attribute

[Green7]

Please check all of the platforms you are having the issue on (if platform is not listed, it is not supported)

• WPF
• UWP
• iOS
• Android
• .NET Standard
• .NET Core

Component

ViewModel

Version of Library

Catel.Core 6.0.1
Catel.Fody 4.9.0

Version of OS(s) listed above with issue

Windows 11

Steps to Reproduce

Here is test code:

  // Simple model class
  public class TestModel : ObservableObject
  {
    private int _coutner;

    public object Property { get; set; }

    private void OnPropertyChanged()
    {
      _coutner++;
      Debug.WriteLine($"Property changed: {Property} {_coutner}");
    }
  }

  // ViewModel 
  public class MyDerivedViewModel : ViewModelBase
  {
    [Model]
    public TestModel Model { get; set; }

    [ViewModelToModel]
    public object Property { get; set; }

    public MyDerivedViewModel()
    {
      Model = new TestModel();
      Model.Property = "test";
    }
  }

We have TestModel object and a ViewModel that uses it. ViewModel exposes the property from the TestModel object by using the attribute [ViewModelToModel].

Expected Behavior

after changing the property function OnPropertyChanged() should be called, one time.

Actual Behavior

OnPropertyChanged() is called twice. And output from creating MyDerivedViewModel is:
Property changed: test 1
Property changed: test 2

If instead of " Model.Property = "test";" we will use the code Property = "test"; OnPropertyChanged is called once

[GeertvanHorrik]

Could it be because it's actually called "Property"? There is an "OnPropertyChanged" virtual method and the property is called OnPropertyChanged. Or does it happen to any sort of property name?

[Green7]

the name of the property does not matter.

[GeertvanHorrik]

I've written reproducable unit tests. Will investigate why this happens.

[GeertvanHorrik]

Generated code:

public object MyProperty
{
	[CompilerGenerated]
	get
	{
		return <MyProperty>k__BackingField;
	}
	[CompilerGenerated]
	set
	{
		<MyProperty>k__BackingField = value;
		OnMyPropertyChanged();
		RaisePropertyChanged("MyProperty");
	}
}

[GeertvanHorrik]

It also seems to happen to Catel 5 (according to the unit tests). It's probably because the ObservableObject does not do a equality check before raising the property.

There could be 2 solutions here (but I think both need to be fixed in Catel itself):

1. Get value before assigning in the model (but it will be "more expensive")
2. Add equality checks in ObservableObject setters (bonus)

[Green7]

Yes, ObservableObject does not check the old value before setting it again.
But I don't understand why the line: Model.Property = "test"; causes the value to be set twice?

[GeertvanHorrik]

It only happens to ObservableObject (not ModelBase) because the ModelBase does equality checks under the hood. I think the best way to solve this is in the ViewModelBase since we can never rely on any model handling this correctly.

[GeertvanHorrik]

Closing this ticket in favor of the Catel one (this will be (is actually) fixed in Catel itself).

[GeertvanHorrik]

btw the other request is covered by #357 , but that will only solve this issue when using the Catel one. If you use any others, it would still fail (hence the fix in Catel itself).

[Green7]

Ok, I'm waiting for a fix in Catel.