« January 2009 | Main | April 2009 »
March 17, 2009
ReadOnlyObservableCollection anti-pattern
I recently fixed a bug that was a result of a subtle misuse of the ReadOnlyObservableCollection<T> class. The code in question was structured as follows: The Source class has a collection of Items. Clients should be able to observe but not modify the collection, so it’s exposed as a ReadOnlyObservableCollection. The Client class adds an event handler to this collection, and removes it at the appropriate time (to prevent a possible memory leak).
class Source
{
Source()
{
m_collection = new ObservableCollection<int>();
}
public ReadOnlyObservableCollection<int> Items
{
get
{
// this is the bug
return new ReadOnlyObservableCollection<int>(m_collection);
}
}
readonly ObservableCollection<int> m_collection;
}
class Client
{
// obtain Source instance from somewhere
Source m_source;
void Subscribe()
{
((INotifyCollectionChanged) m_source.Items).CollectionChanged += SourceItems_CollectionChanged;
}
void Unsubscribe()
{
((INotifyCollectionChanged) m_source.Items).CollectionChanged += SourceItems_CollectionChanged;
}
void SourceItems_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e) { }
}
The bug is that the Source.Items property creates a new ReadOnlyObservableCollection instance each time it’s accessed, which means that the calls to add and remove the event handler happen on two different objects; the event handler is never actually removed.
One solution is for the Client class to cache the exact INotifyCollectionChanged object it subscribed to, so it can be sure to unsubscribe from the same object. The other (and better) solution is for the Source class to create and hold a ReadOnlyObservableCollection, and return the same object each time the Items property is accessed:
public class Source
{
Source()
{
m_collection = new ObservableCollection<int>();
m_collectionReadOnly = new ReadOnlyObservableCollection<int>(m_collection);
}
public ReadOnlyObservableCollection<int> Items
{
get { return m_collectionReadOnly; }
}
readonly ObservableCollection<int> m_collection;
readonly ReadOnlyObservableCollection<int> m_collectionReadOnly;
}
This solves the initial bug and also has the benefit of allowing all clients to share the same ReadOnlyObservableCollection instance (instead of creating one per client).
Posted by Bradley Grainger at 9:02 AM | Comments (2) | TrackBack
March 9, 2009
Subtle Differences
What’s the difference between the values generated by the following statements?
long ticks1 = stopwatch.ElapsedTicks;
long ticks2 = stopwatch.Elapsed.Ticks;
While the latter uses the standard 100-nanosecond intervals (used by FILETIME, DateTime, etc.) as its units, the former reports the number of ticks as measured by the system’s high-resolution performance counter, and is not very meaningful until divided by Stopwatch.Frequency. The use of the noun ‘ticks’ to describe both is unfortunate.
Here’s another difference, which turns out to not be so significant after all:
using System.Linq;
// ...
bool contains1 = dictionary.ContainsKey(value);
bool contains2 = dictionary.Keys.Contains(value);
I initially assumed that the second statement—which invokes the Enumerable.Contains extension method—would perform a linear search over a copy of all the keys in the Dictionary. However, some exploration with Reflector showed that Enumerable.Contains first checks if its parameter implements ICollection<T>. The KeyCollection returned by Dictionary<K, V>.Keys does implement this interface, and its implementation of Contains simply delegates to ContainsKey. Experimentation showed that the latter statement only takes twice as long as the first—not the O(1) vs O(n) difference I was expecting. While API duplication should be avoided, some overlap is inevitable when retrofitting extension methods to an existing codebase; it’s very nice that the LINQ to Objects design allows for an efficient implementation to be chosen when it’s available.
Posted by Bradley Grainger at 8:25 AM | Comments (0) | TrackBack