Changetracking and concurrency - can this fail?

Question

I've been messing around with Expressions - and I may have gone beyond my capabilities - but here goes... I've implemented 'type-safe' INotifyPropertyChanged implementation (an example is here), but gone a bit farther and included changetracking:

public abstract BaseViewModel<TEntity>:INotifyPropertyChanged
{
    private readonly IBaseChangeTracker<TEntity> _changeTracker;
    protected void OnPropertyChanged<T>(Expression<Func<T>> property, T value)
    {
        _changeTracker.AddChange(property, value);
        OnPropertyChanged(property);
    }
    protected virtual void OnPropertyChanged<TProperty>(Expression<Func<TProperty>> property)
    {
        if (PropertyChanged == null) return;
        PropertyChanged(this, new PropertyChangedEventArgs(property.GetMemberInfo().Name));
    }
    public event PropertyChangedEventHandler PropertyChanged;
}

public abstract class BaseChangeTracker<TEntity>:IBaseChangeTracker<TEntity>
{
    private readonly IDictionary<Expression, object> _changes = new Dictionary<Expression, object>();
    public void AddChange<T>(Expression<Func<T>> expression, T newValue)
    {
        _changes.Add(expression, newValue);
    }

    public void ApplyChanges(TEntity entity)
    {
        foreach (var change in _changes)
        {
            var property = typeof(TEntity).GetProperty(change.Key.GetMemberInfo().Name);
            property.SetValue(entity, change.Value, null);
        }
    }

    public virtual void CopyCurrentState(TEntity entity)
    {
        _changes.Clear();

    }
    public virtual void ResetEntity(TEntity entity)
    {
        _changes.Clear();
    }

    public bool HasUnsavedChanges
    {
        get { return _changes.Any(); }
    }
}

This may seem a bit excessive - each entity will have it it's own ChangeTracker keeping the original state of the entity when loaded and can reset it back to these, but the idea is that if there is a concurrency conflict when it tries to save the updated entity, I reload the entity from the database and run it through .ApplyChanges and try to save it again. This will remove about 95% of my concurrency problems... if it works. My tests show that for limited entities it works, but that is with simple property-changes.

Known issue:

1. I have yet to find an elegant way of handling collections.

What else am I missing - or are there obvious flaws in my design?

Answer 1

I realize that this is not a direct answer to your question, but you might consider using the Command Pattern to implement an undo/redo stack.

Encapsulating changes in commands is a very tidy way to cycle/re-cycle through changes, with the added benefits of (1) a nice feature that adds value to the application, (2) you can wrap many actions in any given command, like raising event change notifications for databinding support in both the do and undo directions.

Additionally, managing collection changes is no more or less challenging than simple property updates.

Specific to the code you posted, the OnPropertyChanged implementation will never raise the PropertyChanged event because you call return after the if() statement and then again in bare brackets (these do not correspond to the if condition).

  if (PropertyChanged == null) return; // this returns based on if
    {
        return; // this returns no matter what
    }

Additionally, it seems that the user won't ever see any changes in the UI. The values aren't updated until ApplyChanges is called, and when it is there is no PropertyChanged event. (I might not be following your code correctly, but just looking over it this seems to be the case).