Mark A. Donohoe
Mark A. Donohoe

Reputation: 30488

If you're binding to an object with read-only properties, should you still implement INotifyPropertyChanged?

While writing the API for some components we're developing, we have a need to expose some read-only POCO objects which the user can (and most likely will) create data templates for in their app, thus they will be using bindings to them.

Now I know that you can bind to straight POCO objects (i.e. not subclassed of DependencyObject) even if they don't implement INotifyPropertyChanged but when you do, unless you explicitly mark those bindings as one-time, you're creating unnecessary references to your objects and potentially exposing memory leaks.

Excerpt from this article:

[...] This has 2 implications, this action causes the common language runtime (CLR) to create a strong reference from this PropertyDescriptor object to object X. The CLR also keeps a reference to the PropertyDescriptor object in a global table.

Now while we can try and document that all bindings to these objects should be one-time, that puts the burden on the consumer. I'm wondering if it makes sense to just implement INotifyPropertyChanged anyway, even though nothing ever changes specifically to defensively address the memory leaks.

In other words, which is heavier, binding to a POCO property without using one-time, or implementing INotifyPropertyChanged defensively?

Is there any down-side to doing so? While not as lightweight as one-time bindings, it still has to be better than the effect caused by a consumer not setting them to one-time.

If it makes any difference, there will only be a hundred or so of these so a very small set.

Upvotes: 0

Views: 366

Answers (2)

Glenn Slayden
Glenn Slayden

Reputation: 18839

Yes, you should definitely implement INotifyPropertyChanged for any POCO that might be used with WPF data binding, even if the object's properties never change.

As always with INotifyPropertyChanged (and also in this case), there is no need to fire the event for the initial values which are set in the constructor. This is because, at the time of executing the constructor, there is (normally [1.] ) no possibility of the event having any subscribers yet.

Finally (since it hasn't been mentioned yet on this page), you can avoid the memory waste penalty in this situation by using a vacuous version of the manual event implementation syntax in C#, without declaring any event delegate. Both the explicit and public interface implementation modes are supported, you can choose whichever is your preference:

1. Vacuous event implementation (explicit)

public class MyClass : INotifyPropertyChanged
{
    public int InvariantProperty => 42;

    event PropertyChangedEventHandler INotifyPropertyChanged.PropertyChanged
    {
        add { }
        remove { }
    }
}

2. Vacuous event implementation (public)

public class MyClass : INotifyPropertyChanged
{
    public int InvariantProperty => 42;

    public event PropertyChangedEventHandler PropertyChanged
    {
        add { }
        remove { }
    }
}

Notice that there's no EventHandler delegate declared in the examples, but the important thing is the class still advertizes (inherently moribund) "notifications." Doing so trivially satisfies the WPF binding engine, and thus prevents it from deploying its more invasive and expensive tactics.


1. Of course the unwise bad-practice alluded to here would be calling virtual methods from your constructor--or directly calling into external code--prior to the constructor being finished.

Upvotes: 1

mm8
mm8

Reputation: 169420

Is there any down-side to doing so?

The downside if any should be negligible. There is obviously a very minor memory penalty. Besides that you need to define the PropertyChanged event in your POCO class, WPF will also subscribe to it and this will allocate an extra 4 or 8 bytes depending on your CPU architecture. But this is not really a valid reason no to implement the interface.

So if you are concerned about memory leaks and know for a fact that not all bindings to your class' properties will be defined as OneTime, it does no harm to implement the interface as a guard for this. You may want to leave a comment in the code that mentions why the interface is implemented despite the event never being raised.

Upvotes: 0

Related Questions