Finding Binding Trouble

Homer: There are three ways to do things: the right way, the wrong way, and the Max Power way!
Bart: Isn’t that just the wrong way?
Homer: Yeah, but faster!
– The Simpsons, “Homer to the Max

I was recently doing some performance work in the neighborhood of WPF Items Controls bound to collections where the underlying collection was likely to be swapped out based on behaviors within the application. The exercise got me thinking about the topic and it seems like a good thing to raise some items that are easy to overlook, but most developers should consider…  

First and foremost, there have always been a variety of collection classes in the .Net framework, and they all have specific situations that make them more or less desirable. This extends beyond the difference between Stacks, Queues, Lists, Dictionaries, etc, and includes having to make decisions about how symmetric the ratio will be between reads/writes, the need to sort, the type of value being collected, etc. Choices about which collections to use and how to use them when providing data for databinding in a UI can have a profound effect on performance and usability of the user interface (while this line of thought was started by work in WPF and naturally extends to Silverlight, it can even apply to WinForms and beyond.)  

Within WPF and Silverlight, the INotifyCollectionChanged interface (defined in System.Collections.Specialized) is at the center of data binding with ItemsControls (actually, IBindingList – which is more commonly used with WinForms data binding – is too…) The focus of this interface is to raise a CollectionChanged event whenever something happens that modifies the contents of the collection. The arguments delivered by this event include the type of change (add, remove, replace, move, and reset) and where applicable, information about the collection’s old and/or new state. The most used class that implements this interface in WPF/Silverlight is the ObservableCollection<T>, defined in the System.Collections.ObjectModel namespace.  

I have often come across this code to expose these collections:  

private ObservableCollection<String> firstCollection;
public ObservableCollection<String> FirstCollection
{
     get { return firstCollection; }
     set
     {
          firstCollection = new ObservableCollection<String>(value);
          RaisePropertyChanged("FirstCollection");
    
}
}
I have 4 problems with this code. First, the public property is exposing too much information about the type of backing store that is being used for the underlying collection. Consumers of this property should not care if it uses ObservableCollection<T>, BindingList<T>, or some other custom implementation. Exposing ObservableCollection<T> places an obligation on methods that call the property to provide an ObservableCollection<T> (or a derivative thereof) and makes the codebase difficult to refactor down the road. Encapsulation is broken, etc.  

Second, the property may very well return a null value for the collection at any point. Consumers of the API that would like to iterate over the collection have to remember to add an extra null-check before every attempt to iterate. If they forget such a check, the application is potentially just a few mouse-clicks away from an unhandled exception. It is that much harder for consumers of the API to “fall unavoidably into the pit of success.” I prefer to ensure that and appropriate enumerable value is always returned for these properties, unless null has a very specific meaning (in which case, there may be a better and more expressive way to indicate such a state then with a null value.) Assume the next developer is going to screw things up…and then don’t let him.  

My third concern is probably debatable, but anyway, the collection reference here is not constant. Whenever the property setter is called, the underlying collection is changed to a completely new reference. This requires that a property change notification be raised in order to tell the binding control that it needs to redo the binding, as the object it had bound to is no longer relevant, and neither its content, nor any changes to it should be reflected in the UI (for that matter, it may very well be time to relinquish the resources and space this original item is using.)  

The final item is discussed at the bottom of this post and is slightly orthogonal to CollectionChanged notifications, but is still at play in the code above…it is basically the use of a developer-provided string to provide the property name that is being changed, in the case where a property change notification is necessary as discussed above. The compiler will not help to call out any kind of fat-finger mistake in the typing of the property name, nor will it help if the name of the property is changed but changing the string argument has been overlooked (this is the case even if constants are used…it is still just a string that gets passed to reflection inside of the binding control, and is subject to the same typo and sync issues that can occur elsewhere.)    

Dealing with the First and Second Issues

For starters, instead of returning ObservableCollection<T>, the property should simply return an appropriate abstraction – more often than not (and especially with the power of LINQ-to-objects), the IEnumerable<T> interface is appropriate. Most binding implementations check to see if the provided object implements a well-known interface such as INotifyCollectionChanged and use a cast to that interface anyway. Next, in order to address the possibility of returning null from this layer of the API the getter is changed to trap this condition and return an empty/degenerate value:  

private ObservableCollection<String> firstCollection;
public IEnumerable<String> FirstCollection
{
     get { return firstCollection ?? new ObservableCollection<String>(); }
     set
     {
          firstCollection = new ObservableCollection<String>(value);
          RaisePropertyChanged("FirstCollection");
     }
} 

Providing a Constant Reference to the Collection Object

In order to accomplish this, I used to use the following code:   

private readonly ObservableCollection<String> secondCollection = new ObservableCollection<String>();
public IEnumerable<String> SecondCollection
{
     get { return secondCollection; }
     set
     {
          secondCollection.Clear();
          if (value != null)
          {
               // An equivalent extension method to the code that follows can be used.
               // secondCollection.AddRange(value);
               foreach (var item in value)
               {
                    secondCollection.Add(item);
               }
          }
     }
}  

This approach was quick to implement, and fairly clear to follow. For small collections and simple situations, it worked fairly well. However, there is one big performance-related problem with this approach. The ObservableCollection<T> dutifully raises a CollectionChanged event every time an item is added inside the “AddRange” loop, which causes the binding control to respond, etc. In simple observations (binding a ListBox to a collection of 10,000 Strings), the performance hit is between 5x and 10x.   

To overcome this gaffe, it is necessary to be able to use an ObservableCollection<T> implementation that can say “when I am swapping out the collections’ contents, don’t raise an event every time an item is added, but instead raise a single event at the end of the operation that indicates the whole collection has been reset.” At least one third party control-company that I am aware of includes something like this in their toolkit, but implementing a simplified version yourself is a fairly straightforward process:   

  • Create a new class that derives from ObservableCollection<T>
  • Add a Boolean variable called suppressNotifications
  • Provide a ResetContents method as follows:
    • Note the suppressNotifications value is flagged when starting the operations, and then turned back off when done, followed by a Reset CollectionChanged notification.
public void ResetContents(IEnumerable<T> items)
{
     if (items == nullthrow new ArgumentNullException("items");
     try
     {
          suppressNotifications = true;
          ClearItems();
          foreach(var item in items)
          {
               Add(item);
          }
     }
     finally
     {
          suppressNotifications = false;
          OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
      }
}  
  • Provide overrides for the ClearItems and InsertItem virtual methods (defined in Collection<T>), which are used internally within the Clear and Add methods, respectively.
    • ObservableCollection<T> implements overrides for these that raise the events we want to avoid for now.
    • Note that if events are not being suppressed, the collection just defers to the base implementation from ObservableCollection<T>. Otherwise, it just checks for reentrancy and then proceeds to insert items without raising any events.
protected override void ClearItems()
{
     if (suppressNotifications)
     {
          CheckReentrancy();
          Items.Clear();
     }
     else
     {
          base.ClearItems();
     }
}

protected override void InsertItem(int index, T item)
{
     if (suppressInsertEvent)
     {
          CheckReentrancy();
          Items.Insert(index, item);
     }
     else
     {
          base.InsertItem(index, item);
     }
}

For this implementation, performance is again on par with that seen when the entire collection is swapped out with and a PropertyChange notification is raised. The final implementation is as follows (note that because an IEnumerabe<T> is being returned, the actual backing type is irrelevant to the consuming code:   

private readonly EnhancedObservableCollection<String> thirdCollection 
new EnhancedObservableCollection<String>();
public IEnumerable<String> ThirdCollection
{
     get { return thirdCollection; }
     set
     {
          thirdCollection.Clear();
          if (value != null)
          {
               thirdCollection.ResetContents(value);
          }
     }
}

While these three changes required a little extra (boilerplate) work, the net result is code that the consumers of this API will have a much harder time using incorrectly:   

  • They do not have to be concerned with the precise backing type, and the result is Linq-friendly if they want to find out about its contents (eg call Count(), etc.)
  • They can safely iterate over the collection without worrying about whether it has been set to null.
  • They can take and hold a reference to the collection and not worry about whether or not some other code has called the setter, leaving a stale reference.

One Final Note – Property Change Notifications

Back to the issue about “magic strings” and the INotifyPropertyChanged interface. Even in .Net 2, developers sometimes went to some lengths to provide some level of either compile-time checking or debug-mode assistance. Options I have seen include using reflection to determine the name of the property that is calling the “RaiseXXX” helper function, using reflection only when in debug mode to verify that the name provided maps to an actual property and raising an assertion to try to make the issue more obvious during testing, using constants or enumerations for the string values, etc. With .Net 3 and LINQ constructs (specifically Expression Trees), there is a new and elegant solution that involves the compiler in the process of ensuring that valid entries are provided, and though it does have a performance impact, it is important to remember that when binding, reflection is usually used from the item doing the binding anyway, which brings along its own set of performance considerations/limitations. Wintellect’s
Jeremy Likness has a great, detailed, and more complete writeup (required reading!) on this approach, but here is a simplified implementation:   

public event PropertyChangedEventHandler PropertyChanged;
private void RaisePropertyChanged<T>(Expression<Func<T>> propertyBeingChanged)
{
     if (propertyBeingChanged == nullthrow new ArgumentNullException("propertyBeingChanged");
     var memberExpression = propertyBeingChanged.Body as MemberExpression;
     if (memberExpression == nullthrow new ArgumentException("propertyBeingChanged");
     String propertyName = memberExpression.Member.Name;
     var tempPropertyChanged = PropertyChanged;
     if (tempPropertyChanged != null)
     {
          tempPropertyChanged(thisnew PropertyChangedEventArgs(propertyName));
     }
}

And the call to raise the property change notification, in the context of our original property implementation, above:   

private ObservableCollection<String> firstCollection;
public IEnumerable<String> FirstCollection
{
     get { return firstCollection ?? new ObservableCollection<String>(); }
     set
     {
          firstCollection = new ObservableCollection<String>(value);
          RaisePropertyChanged(() => FirstCollection);
     }
}
Notice the Lambda function used in the call to RaisePropertyChanged. Miss the property name by so much as a letter (assuming no other property matches the new name), and the compiler will protest. This does come at a performance price, however. In my tests, the "Compiler-Safe" option can take as much as 200x as the "magic string" version, which on my dev laptop was the difference between five one-hundredths of a second and one full second to raise 100,000 PropertyChanged event notifications. How big of a concern this is may largely depend on how many property changes you anticipate need to be completed within your bound data, which brings us back to the original discussion about choosing collections wisely in one's application implementations. If your properties are likely to change only through user interaction in the UI, there's probably a reduced need to worry about millisecond-range performance issues like these and to favor an implementation that reduces the amount of run-time debugging that will be required (but it is still important to know about and evaluate these issues.)   

Sample Code Used for Timings
Advertisement

2 thoughts on “Finding Binding Trouble

  1. Pingback: John Garland's Blog : Using the New CallerInfo Attributes for Reliable Property Change Notifications

  2. Pingback: Using the New Caller Information Attributes for Reliable Property Change Notifications « DotNet Gator

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s