Runtime: Proposal: Add OldValue and NewValue properties in PropertyChangedEventArgs.

Created on 28 Aug 2018  路  7Comments  路  Source: dotnet/runtime

Sometime there is a need to subscribe to INotifyPropertyChanged.PropertyChanged event of a model from another view model, e.g. logging of property changes of the model from the view model. Current PropertyChangedEventArgs does not have a OldValue and NewValue property for this purpose.

Proposal: Add OldValue & NewValue properties

public class PropertyChangedEventArgs : EventArgs
{
    // Properties.
    public string PropertyName { get; set; }
    public object OldValue { get; set; }
    public object NewValue { get; set; }

    // ctor.
    public PropertyChangedEventArgs(string propertyName, object oldValue = null, object newValue = null)
    {
        PropertyName = propertyName;
        OldValue = oldValue;
        NewValue = newValue;
    }
}

With the above changes, it will be possible for the model to raise PropertyChanged event in the property setter.

public class Foo : INotifyPropertyChanged
{
    // Fields.
    private DateTime m_CurrentDateTime;
    // Binding Properties.
    public DateTime CurrentDateTime
    {
        get
        {
            return m_CurrentDateTime;
        }
        set
        {
            if (!DateTime.Equals(m_CurrentDateTime, value))
            {
                var oldValue = m_CurrentDateTime;
                m_CurrentDateTime = value;
                OnPropertyChanged(oldValue, value);
            }
        }
    }
    // Methods.
    private void OnPropertyChanged(object oldValue, object newValue, [CallerMemberName]string propertyName = "")
    {
        var arg = new PropertyChangedEventArgs(propertyName, oldValue, newValue);
        PropertyChanged?.Invoke(this, arg);
    }

    // ctor.
    public Foo()
    {
    }

    // INotifyPropertyChange.
    public event PropertyChangedEventHandler PropertyChanged;
}

Cross reference from here.

api-suggestion area-System.ComponentModel

Most helpful comment

Would a new subclass, PropertyChangedEventArgs<T> be a better idea?

That would remove the need for boxing and would not grow the size of the object for existing users.

All 7 comments

Would a new subclass, PropertyChangedEventArgs<T> be a better idea?

That would remove the need for boxing and would not grow the size of the object for existing users.

@tannergooding That will means the following

public class PropertyChangedEventArgs<T> : EventArgs
{
    // Properties.
    public string PropertyName { get; set; }
    public T OldValue { get; set; }
    public T NewValue { get; set; }

    // ctor.
    public PropertyChangedEventArgs(T oldValue, T newValue, [CallerMemberName]string propertyName = "")
    {
        PropertyName = propertyName;
        OldValue = oldValue;
        NewValue = newValue;
    }
}

which leads to

public delegate void PropertyChangedEventHandler<T>(object sender, PropertyChangedEventArgs<T> arg);

public interface INotifyPropertyChanged<T>
{
    event PropertyChangedEventHandler<T> PropertyChanged;
}

Since INotifyPropertyChanged has become a generic interface, the implementer will need to implement multiple generic interfaces with different type, which may look overkill.

Other possible issues with generic interface are also mentioned here.

Why would you have to implement a generic interface? You could continue using regular INotifyPropertyChanged and just type check and cast the event args.

@tannergooding Looking back at your previous comment, you'd mentioned subclass, which I obviously have missed out. So the subclass will be

public class PropertyChangedEventArgs<T> : System.ComponentModel.PropertyChangedEventArgs
{
    // Properties.
    public T OldValue { get; set; }
    public T NewValue { get; set; }

    // ctor.
    public PropertyChangedEventArgs(T oldValue, T newValue, [CallerMemberName]string propertyName = "")
        :base(propertyName)
    {
        OldValue = oldValue;
        NewValue = newValue;
    }
}

Event handler at the view model has to cast the event argument accordingly.

// Eventhandlers.
private void OnPropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e)
{
    switch (e)
    {
        case PropertyChangedEventArgs<DateTime> arg:
            // Logs is a list of string.
            Logs.Add($"{e.PropertyName} | {arg.OldValue} -> {arg.NewValue}");
            break;
    }
}

The (probably) downside is that there will be multiple cases that perform the same operation (e.g. adding log). Probably there is a better way to do that?

I would imagine your code would look something like:

private void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
{
    switch (e.PropertyName)
    {
        case "Property1":
            OnProperty1Changed(sender, (PropertChangedEventArgs<T1>)e);
            break;

        case "Property2":
            OnProperty2Changed(sender, (PropertChangedEventArgs<T2>)e);
            break;
    }
}

The overall point, however, is to avoid the need for the "generic explosion" that would otherwise happen, and to allow existing code to continue working without any hits (so it is a pay-for-play model):

  • No increased size for OldValue/NewValue unless you need them
  • No boxing/unboxing required for the values
  • Provided you are only firing the events internally, you can ensure PropertyChangedEventArgs<T> is always used (so you don't need to worry about handling both cases)
  • etc

This is entirely possible to implement with extension methods and the existing event, albeit with a bit more memory usage, and honestly I'd rather not see it become a core library part because then people would need to write code handling both the new and the old ways of doing it for years because adding new API doesn't magically update all users of the old API.

Additionally short of adding another interface(Which brings us back to having to deal with inconsistent usage), there is no way to distinguish whether a particular class supports the new event class from the code signature.

Also currently if you supply "null" or "String.Empty" as an argument to the current PropertyChanged event, it indicates that all properties have changed, and that behavior would be annoying/difficult to do in your proposed event structure.

This is how I do it internally, the only major drawback is that it does create a storage of previous value for each listener rather than for each event.:

```c#
public static class PropertyChangedExtensions
{
....
///


/// Watch an item for changes to a specific property
///

/// The item to listen to
/// The property to watch on said item
/// The action to perform when the property changes
/// This parameter is optional and only really needed in high volume code paths, expression compilation is somewhat expensive
/// A disposable handle that will unwatch the event when disposed
public static IDisposable OnChange(this T toListenTo, Expression> propertyExpression, Action> action, Func getProperty = null)
where T : INotifyPropertyChanged
{
return new INotifyPropertyChangedDetatchable(toListenTo, GetMemberName(propertyExpression), getProperty ?? propertyExpression.Compile(), action);
}

....

    private static string GetMemberName<T, TProperty>(Expression<Func<T, TProperty>> propertyExpression)
    {
        return (propertyExpression?.Body as MemberExpression)?.Member.Name;
    }

    private class INotifyPropertyChangedDetatchable<T, TProperty> : IDisposable
        where T : INotifyPropertyChanged
    {
        private readonly T ToListenTo;
        private readonly string Property;
        private readonly Action Action;
        private TProperty Previous;

....
public INotifyPropertyChangedDetatchable(T sender, string property, Func getVal, Action> action)
{
ToListenTo = sender;
Property = property;
Previous = getVal(ToListenTo);
Action = () =>
{
TProperty current = getVal(ToListenTo);
action(new OldAndNew(Previous, current));
Previous = current;
};
ToListenTo.PropertyChanged += ToListenTo_PropertyChanged;
}
....

        void ToListenTo_PropertyChanged(object sender, PropertyChangedEventArgs e)
        {
            if (Property == null || e.PropertyName == null || e.PropertyName == Property)
                Action();
        }

        public void Dispose()
        {
            ToListenTo.PropertyChanged -= ToListenTo_PropertyChanged;
        }
    }

....
}
....
public class OldAndNew
{
public readonly T OldVal;
public readonly T NewVal;

    public OldAndNew(T oldVal, T newVal)
    {
        OldVal = oldVal;
        NewVal = newVal;
    }
}

```

I have written the same thing myself; and while very useful, I think it is a little bloated to include in the BCL --- there needs to be some machinery somewhere to implement the value handling; and then some policy is involved. The "property value swapped" pattern seems a bit far to go to say that the BCL should provide some type of support.

As @DanFTRX pointed out, it's easily implementable by some local or utility patterns (where still machinery and an added value reference are involved).

I implemented mine with an Attribute and a base class; and it does Liskov nice and transparently --- the consumer can simply know about it or don't --- and refactoring is not made any more complicated. And though the base class is a good place to put the responsibility, the extender actually will add it even where not implemented. [But the base class can also define the value generically.]

public class TypedPropertyChangedAttribute
        : Attribute;
public class TypedPropertyChangedEventArgs<TValue>
        : PropertyChangedEventArgs
{
    public TValue Arg { get; }
}

public class MyObject
        : BindableDataContract
{
    [TypedPropertyChanged]
    public int Value
    {
        get => _value; 
        set => SetProperty(ref _value, value);
    }
}

public class BindableDataContract
{
    protected bool IsTypedPropertyChanged(string propertyName);
    protected void SetValue<T>(
            ref T member,
            T value,
            [CallerMemberName] string propertyName = null)
    {
        if (IsTypedPropertyChanged(propertyName)
            // Capture value and raise TypedPropertyChangedEventArgs
    }
}

... The consumer has to cast the incoming PropertyChangedEventArgs ...

private void handleEvent(object sender, PropertyChangedEventArgs eventArgs)
{
    if (eventArgs is TypedPropertyChangedEventArgs<int> typedEvent) {
        int priorValue = typedEvent.Arg;
    }
}
Was this page helpful?
0 / 5 - 0 ratings

Related issues

matty-hall picture matty-hall  路  3Comments

Timovzl picture Timovzl  路  3Comments

aggieben picture aggieben  路  3Comments

jchannon picture jchannon  路  3Comments

nalywa picture nalywa  路  3Comments