Automatic redrawing in WPF

Question

I would like to re-render my WPF chart control when data source is changed. My minimum requirement for a data source is IEnumerable<Point> but it could be also ObservableCollection<Point> or BindingList<Point>, so collection change notifications are sometimes becoming available.

Chart Serie class (simplified):

class Serie : Freezable
{
    public static readonly DependencyProperty DataProperty =
        DependencyProperty.Register("Data", typeof(IEnumerable<Point>), typeof(Serie));

    public Serie()
    {
        new ChangeListner(this, DataProperty);
    }

    public IEnumerable<Point> Data
    {
        get { return (IEnumerable<Point>)GetValue(DataProperty); }
        set { SetValue(DataProperty, value); }
    }

    public void Draw(DrawingContext drawingContext)
    {
        if (Data == null)
            return;

        foreach (var point in Data)
            drawingContext.DrawEllipse(Brushes.Black, null, point, 4, 4);
    }

    protected override Freezable CreateInstanceCore() => new Serie();
}

Where notifications are being monitored by (real code):

class ChangeListner
{
    public ChangeListner(Freezable subject, DependencyProperty property)
    {
        Subject = subject;
        Property = property;
        DependencyPropertyDescriptor
            .FromProperty(property, subject.GetType())
            .AddValueChanged(subject, OnChanged);
    }

    private void OnChanged(object sender, EventArgs e)
    {
        var value = Subject.GetValue(Property);
        BindingList = value as IBindingList;
        NotifyCollectionChanged = value as INotifyCollectionChanged;
        NotifyPropertyChanged = value as INotifyPropertyChanged;
    }

    Freezable Subject { get; }
    DependencyProperty Property { get; }

    INotifyPropertyChanged _notifyPropertyChanged;
    INotifyPropertyChanged NotifyPropertyChanged
    {
        set
        {
            if (_notifyPropertyChanged != null)
                _notifyPropertyChanged.PropertyChanged -= _notifyPropertyChanged_CollectionChanged;

            _notifyPropertyChanged = value;

            if (_notifyPropertyChanged != null)
                _notifyPropertyChanged.PropertyChanged += _notifyPropertyChanged_CollectionChanged;
        }
    }
    void _notifyPropertyChanged_CollectionChanged(object sender, PropertyChangedEventArgs e) =>
        Notify();

    INotifyCollectionChanged _notifyCollectionChanged;
    INotifyCollectionChanged NotifyCollectionChanged
    {
        set
        {
            if (_notifyCollectionChanged != null)
                _notifyCollectionChanged.CollectionChanged -= _notifyCollectionChanged_CollectionChanged;

            _notifyCollectionChanged = value;

            if (_notifyCollectionChanged != null)
                _notifyCollectionChanged.CollectionChanged += _notifyCollectionChanged_CollectionChanged;
        }
    }
    void _notifyCollectionChanged_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e) =>
        Notify();

    IBindingList _bindingList;
    IBindingList BindingList
    {
        set
        {
            if (_bindingList != null)
                _bindingList.ListChanged -= _bindingList_ListChanged;

            _bindingList = value;

            if (_bindingList != null)
                _bindingList.ListChanged += _bindingList_ListChanged;
        }
    }
    void _bindingList_ListChanged(object sender, ListChangedEventArgs e) =>
        Notify();

    void Notify() => Subject
        .GetType()
        .GetMethod("NotifySubPropertyChange", BindingFlags.Instance | BindingFlags.NonPublic)
        .Invoke(Subject, new[] { Property });
}

I am not quite sure about this Notify method – reflection invocation is slow and dangerous. Is there a better way to make Freezable pickup changes?

Here is the custom control code I use for testing:

class Plot : Control
{
    public static readonly DependencyProperty SeriesProperty =
        DependencyProperty.Register("Series", typeof(FreezableCollection<Serie>), typeof(Plot),
            new FrameworkPropertyMetadata(null, FrameworkPropertyMetadataOptions.AffectsRender));

    public Plot()
    {
        Series = new FreezableCollection<Serie>();
    }

    public FreezableCollection<Serie> Series
    {
        get { return (FreezableCollection<Serie>)GetValue(SeriesProperty); }
        set { SetValue(SeriesProperty, value); }
    }

    protected override void OnRender(DrawingContext drawingContext)
    {
        base.OnRender(drawingContext);
        if (Series == null)
            return;

        foreach (var serie in Series)
            serie.Draw(drawingContext);
    }
}

Answer 1

Here is how to make it quick (many thanks to @dymanoid). The use of internal method is still questionable...

class ChangeListner
{
    public ChangeListner(Freezable subject, DependencyProperty property)
    {
        Subject = subject;
        Property = property;

        DependencyPropertyDescriptor
            .FromProperty(property, subject.GetType())
            .AddValueChanged(subject, OnChanged);

        NotifySubPropertyChange = (Action<DependencyProperty>)Delegate.CreateDelegate(
            typeof(Action<DependencyProperty>),
            subject,
            "NotifySubPropertyChange");
    }

    Freezable Subject { get; }
    DependencyProperty Property { get; }
    Action<DependencyProperty> NotifySubPropertyChange { get; }

    void OnChanged(object sender, EventArgs e)
    {
        var value = Subject.GetValue(Property);
        BindingList = value as IBindingList;
        NotifyCollectionChanged = value as INotifyCollectionChanged;
        NotifyPropertyChanged = value as INotifyPropertyChanged;
    }               

    INotifyPropertyChanged _notifyPropertyChanged;
    INotifyPropertyChanged NotifyPropertyChanged
    {
        set
        {
            if (_notifyPropertyChanged != null)
                _notifyPropertyChanged.PropertyChanged -= _notifyPropertyChanged_CollectionChanged;

            _notifyPropertyChanged = value;

            if (_notifyPropertyChanged != null)
                _notifyPropertyChanged.PropertyChanged += _notifyPropertyChanged_CollectionChanged;
        }
    }
    void _notifyPropertyChanged_CollectionChanged(object sender, PropertyChangedEventArgs e) =>
        NotifySubPropertyChange(Property);

    INotifyCollectionChanged _notifyCollectionChanged;
    INotifyCollectionChanged NotifyCollectionChanged
    {
        set
        {
            if (_notifyCollectionChanged != null)
                _notifyCollectionChanged.CollectionChanged -= _notifyCollectionChanged_CollectionChanged;

            _notifyCollectionChanged = value;

            if (_notifyCollectionChanged != null)
                _notifyCollectionChanged.CollectionChanged += _notifyCollectionChanged_CollectionChanged;
        }
    }
    void _notifyCollectionChanged_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e) =>
        NotifySubPropertyChange(Property);

    IBindingList _bindingList;
    IBindingList BindingList
    {
        set
        {
            if (_bindingList != null)
                _bindingList.ListChanged -= _bindingList_ListChanged;

            _bindingList = value;

            if (_bindingList != null)
                _bindingList.ListChanged += _bindingList_ListChanged;
        }
    }
    void _bindingList_ListChanged(object sender, ListChangedEventArgs e) =>
        NotifySubPropertyChange(Property);
}

Answer 2

I find the coding style is somewhat unconventional, which might make maintenance more costly than it needs to be. Particular smells are:

1. Invocation of a constructor where the object is not used.

           new ChangeListner(this, DataProperty);
   

   I would find this more readable as a static method ChangeListener.Register (note the extra e in Listener!). Then the listener would be something like

       public ChangeListener(Freezable subject, DependencyProperty property)
       {
           Subject = subject;
           Property = property;
       }
   
       public static void Register(Freezable subject, DependencyProperty property)
       {
           var listener = new ChangeListener(subject, property);
           DependencyPropertyDescriptor
               .FromProperty(property, subject.GetType())
               .AddValueChanged(subject, listener.OnChanged);
       }
   

2. Use of setter-only properties as methods. This can occasionally make sense where you need a property for binding purposes, but in general it's more obvious for methods to be methods.

I'd almost add a third, but it's more at the level of a question than a code smell. Why does ChangeListener care about the subject being Freezable? I can't see an obvious reason why Subject's type isn't DependencyObject.

---

In answer to your question as to alternative ways to make your subject pick up the changes: there are two options which I'd use before reflection. The difference between them is the direction of control.

Option 1: callback. Make the static Register method take a third argument, probably of type Action, and call that action instead of Notify().

Option 2: event. Give ChangeListener an event EventHandler SubPropertyChanged invoked by

    private void Notify() => SubPropertyChanged?.Invoke(Subject, EventArgs.Empty);

Then the Serie can register a handler which notifies its own public event.