Recently I inherited a pretty big project developed in C# and WPF.
It uses bindings along with the INotifyPropertyChanged interface to propagate changes to/from the View.
A little preface:
In different classes I have properties that depend on other properties in the same class (think for example the property TaxCode that depends on properties like Name and Lastname).
With the help of some code I found here on SO (can't find again the answer though) I created the abstract class ObservableObject and the attribute DependsOn.
The source is the following:
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
namespace TestNameSpace
{
[AttributeUsage(AttributeTargets.Property, Inherited = false)]
public sealed class DependsOn : Attribute
{
public DependsOn(params string[] properties)
{
this.Properties = properties;
}
public string[] Properties { get; private set; }
}
[Serializable]
public abstract class ObservableObject : INotifyPropertyChanged
{
private static Dictionary<Type, Dictionary<string, string[]>> dependentPropertiesOfTypes = new Dictionary<Type, Dictionary<string, string[]>>();
[field: NonSerialized]
public event PropertyChangedEventHandler PropertyChanged;
private readonly bool hasDependentProperties;
public ObservableObject()
{
DependsOn attr;
Type type = this.GetType();
if (!dependentPropertiesOfTypes.ContainsKey(type))
{
foreach (PropertyInfo pInfo in type.GetProperties())
{
attr = pInfo.GetCustomAttribute<DependsOn>(false);
if (attr != null)
{
if (!dependentPropertiesOfTypes.ContainsKey(type))
{
dependentPropertiesOfTypes[type] = new Dictionary<string, string[]>();
}
dependentPropertiesOfTypes[type][pInfo.Name] = attr.Properties;
}
}
}
if (dependentPropertiesOfTypes.ContainsKey(type))
{
hasDependentProperties = true;
}
}
public virtual void OnPropertyChanged(string propertyName)
{
this.PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
if (this.hasDependentProperties)
{
//check for any computed properties that depend on this property
IEnumerable<string> computedPropNames = dependentPropertiesOfTypes[this.GetType()].Where(kvp => kvp.Value.Contains(propertyName)).Select(kvp => kvp.Key);
if (computedPropNames != null && !computedPropNames.Any())
{
return;
}
//raise property changed for every computed property that is dependant on the property we did just set
foreach (string computedPropName in computedPropNames)
{
//to avoid stackoverflow as a result of infinite recursion if a property depends on itself!
if (computedPropName == propertyName)
{
throw new InvalidOperationException("A property can't depend on itself");
}
this.OnPropertyChanged(computedPropName);
}
}
}
protected bool SetField<T>(ref T field, T value, [CallerMemberName] string propertyName = null)
{
return this.SetField<T>(ref field, value, false, propertyName);
}
protected bool SetField<T>(ref T field, T value, bool forceUpdate, [CallerMemberName] string propertyName = null)
{
bool valueChanged = !EqualityComparer<T>.Default.Equals(field, value);
if (valueChanged || forceUpdate)
{
field = value;
this.OnPropertyChanged(propertyName);
}
return valueChanged;
}
}
}
These classes allow me to:
this.SetValue(ref this.name, value) inside the setter of my properties.DependsOn(nameof(Name), nameof(LastName)) on the property TaxCodeThis way TaxCode only has a getter property that combines FirstName, LastName (and other properties) and returns the corresponding code. Even with binding this property is up to date thanks to this dependency system.
So, as long as TaxCode has dependencies on properties that are in the same class, everything works correctly. However I'm in the need to have properties that have one or more dependencies on their child object. For example (I'll just use json to make the hierarchy more simple):
{
Name,
LastName,
TaxCode,
Wellness,
House:
{
Value
},
Car:
{
Value
}
}
So, the Property Wellness of person sould be implemented like this:
[DependsOn(nameof(House.Value), nameof(Car.Value))]
public double Wellness { get =>(this.House.Value + this.Car.Value);}
The first problem is that "House.Value" and "Car.Value" are not valid parameters for nameof in that context.
The second is that with my actual code I can raise properties that are only in the same object so no properties of childs, nor properties that are application wide (I have for example a property that represents if the units of measurement are expressed in metric/imperial and the change of it affects how values are shown).
Now a solution I could use could be to insert a dictionary of events in my ObservableObject with the key being the name of the property and make the parent register a callback. This way when the property of a child changes the event is fired with the code to notify that a property in the parent has changed. This approach however forces me to register the callbacks everytime a new child is instantiated. It is certainly not much, but I liked the idea of just specifying dependencies and let my base class do the work for me.
So, long story short, what I'm trying to achieve is to have a system that can notify dependent property changes even if the properties involved are its childs or are unrelated to that specific object. Since the codebase is quite big I'd like not to just throw away the existing ObservableObject + DependsOn approach, and I'm looking for a more elegant way than just place callbacks all over my code.
Of course If my approach is wrong / what I want cannot be achieved with the code I have, please DO feel free to suggest better ways.
The original solution with a DependsOnAttribute is a nice idea, but the implementation has a couple of performance and multithreading issues. Anyway, it doesn't introduce any surprising dependencies to your class.
class MyItem : ObservableObject
{
public int Value { get; }
[DependsOn(nameof(Value))]
public int DependentValue { get; }
}
Having this, you can use your MyItem anywhere - in your app, in unit tests, in a class library you might be willing to create later.
Now, consider such a class:
class MyDependentItem : ObservableObject
{
public IMySubItem SubItem { get; } // where IMySubItem offers some NestedItem property
[DependsOn(/* some reference to this.SubItem.NestedItem.Value*/)]
public int DependentValue { get; }
[DependsOn(/* some reference to GlobalSingleton.Instance.Value*/)]
public int OtherValue { get; }
}
This class has two "surprising" dependencies now:
MyDependentItem now needs to know a particular property of the IMySubItem type (whereas originally, it only exposes an instance of that type, without knowing its details). When you change the IMySubItem properties somehow, you are forced to change the MyDependentItem class too.
Additionally, MyDependentItem needs a reference to a global object (represented as a singleton here).
All this breaks the SOLID principles (it's all about to minimize changes in code) and makes the class not testable. It introduces a tight coupling to other classes and lowers the class' cohesion. You will have troubles debugging the issues with that, sooner or later.
I think, Microsoft faced same issues when they designed the WPF Data Binding Engine. You're somehow trying to reinvent it - you're looking for a PropertyPath as it is currently being used in XAML bindings. To support this, Microsoft created the whole dependency property concept and a comprehensive Data Binding Engine that resolves the property paths, transfers the data values and observes the data changes. I don't think you really want something of that complexity.
Instead, my suggestions would be:
For the property dependencies in the same class, use the DependsOnAttribute as you're currently doing. I would slightly refactor the implementation to boost the performance and to ensure the thread safety.
For a dependency to an external object, use the Dependency Inversion Principle of SOLID; implement it as dependency injection in constructors. For your measurement units example, I would even separate the data and the presentation aspects, e.g. by using a view-model that has a dependency to some ICultureSpecificDisplay (your measurement units).
class MyItem
{
public double Wellness { get; }
}
class MyItemViewModel : INotifyPropertyChanged
{
public MyItemViewModel(MyItem item, ICultureSpecificDisplay display)
{
this.item = item;
this.display = display;
}
// TODO: implement INotifyPropertyChanged support
public string Wellness => display.GetStringWithMeasurementUnits(item.Wellness);
}
If I still didn't convince you - well, you can of course extend your DependsOnAttribute to store not only property names but also the types where those properties are declared. Your ObservableObject needs to be updated too.
Let's take a look. This is an extended attribute that also can hold the type reference. Note that it can be applied multiple times now.
[AttributeUsage(AttributeTargets.Property, AllowMultiple = true)]
class DependsOnAttribute : Attribute
{
public DependsOnAttribute(params string[] properties)
{
Properties = properties;
}
public DependsOnAttribute(Type type, params string[] properties)
: this(properties)
{
Type = type;
}
public string[] Properties { get; }
// We now also can store the type of the PropertyChanged event source
public Type Type { get; }
}
The ObservableObject needs to subscribe to the children events:
abstract class ObservableObject : INotifyPropertyChanged
{
// We're using a ConcurrentDictionary<K,V> to ensure the thread safety.
// The C# 7 tuples are lightweight and fast.
private static readonly ConcurrentDictionary<(Type, string), string> dependencies =
new ConcurrentDictionary<(Type, string), string>();
// Here we store already processed types and also a flag
// whether a type has at least one dependency
private static readonly ConcurrentDictionary<Type, bool> registeredTypes =
new ConcurrentDictionary<Type, bool>();
protected ObservableObject()
{
Type thisType = GetType();
if (registeredTypes.ContainsKey(thisType))
{
return;
}
var properties = thisType.GetProperties()
.SelectMany(propInfo => propInfo.GetCustomAttributes<DependsOn>()
.SelectMany(attribute => attribute.Properties
.Select(propName =>
(SourceType: attribute.Type,
SourceProperty: propName,
TargetProperty: propInfo.Name))));
bool atLeastOneDependency = false;
foreach (var property in properties)
{
// If the type in the attribute was not set,
// we assume that the property comes from this type.
Type sourceType = property.SourceType ?? thisType;
// The dictionary keys are the event source type
// *and* the property name, combined into a tuple
dependencies[(sourceType, property.SourceProperty)] =
property.TargetProperty;
atLeastOneDependency = true;
}
// There's a race condition here: a different thread
// could surpass the check at the beginning of the constructor
// and process the same data one more time.
// But this doesn't really hurt: it's the same type,
// the concurrent dictionary will handle the multithreaded access,
// and, finally, you have to instantiate two objects of the same
// type on different threads at the same time
// - how often does it happen?
registeredTypes[thisType] = atLeastOneDependency;
}
public event PropertyChangedEventHandler PropertyChanged;
protected void OnPropertyChanged(string propertyName)
{
var e = new PropertyChangedEventArgs(propertyName);
PropertyChanged?.Invoke(this, e);
if (registeredTypes[GetType()])
{
// Only check dependent properties if there is at least one dependency.
// Need to call this for our own properties,
// because there can be dependencies inside the class.
RaisePropertyChangedForDependentProperties(this, e);
}
}
protected bool SetField<T>(
ref T field,
T value,
[CallerMemberName] string propertyName = null)
{
if (EqualityComparer<T>.Default.Equals(field, value))
{
return false;
}
if (registeredTypes[GetType()])
{
if (field is INotifyPropertyChanged oldValue)
{
// We need to remove the old subscription to avoid memory leaks.
oldValue.PropertyChanged -= RaisePropertyChangedForDependentProperties;
}
// If a type has some property dependencies,
// we hook-up events to get informed about the changes in the child objects.
if (value is INotifyPropertyChanged newValue)
{
newValue.PropertyChanged += RaisePropertyChangedForDependentProperties;
}
}
field = value;
OnPropertyChanged(propertyName);
return true;
}
private void RaisePropertyChangedForDependentProperties(
object sender,
PropertyChangedEventArgs e)
{
// We look whether there is a dependency for the pair
// "Type.PropertyName" and raise the event for the dependent property.
if (dependencies.TryGetValue(
(sender.GetType(), e.PropertyName),
out var dependentProperty))
{
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(dependentProperty));
}
}
}
You can use that code like this:
class MyClass : ObservableObject
{
private int val;
public int Val
{
get => val;
set => SetField(ref val, value);
}
// MyChildClass must implement INotifyPropertyChanged
private MyChildClass child;
public MyChildClass Child
{
get => child;
set => SetField(ref child, value);
}
[DependsOn(typeof(MyChildClass), nameof(MyChildClass.MyProperty))]
[DependsOn(nameof(Val))]
public int Sum => Child.MyProperty + Val;
}
The Sum property depends on the Val property of the same class and on the MyProperty property of the MyChildClass class.
As you see, this doesn't look that great. Furthermore, the whole concept depends on the event handler registration performed by the property setters. If you happen to set the field value directly (e.g. child = new MyChildClass()), then it all won't work. I would suggest you not to use this approach.