HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

ThreadSafeObservableCollection of (T)

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
threadsafeobservablecollectionstackoverflowprogramming

Problem

The idea here is to implement a simple, threadsafe, observable collection that clients can bind to, whilst background threads can update. Changes in the contained items raise the CollectionChanged event.

Feedback appreciated:

```
public class ThreadSafeObservableCollection : INotifyCollectionChanged
where T: INotifyPropertyChanged
{
private readonly Object _lock = new Object();
private readonly Collection _items = new Collection();
private ReadOnlyCollection _readonlyItems;

public ThreadSafeObservableCollection()
{

}

public ThreadSafeObservableCollection(IEnumerable items)
{
_items = new ObservableCollection(items);
foreach (T item in _items)
{
((INotifyPropertyChanged) item).PropertyChanged += ItemPropertyChangedHandler;
}
}

public ReadOnlyCollection Items
{
get { return _readonlyItems ?? (_readonlyItems = new ReadOnlyCollection(_items)); }
}

public virtual void Add(T obj)
{
lock(_lock)
{
((INotifyPropertyChanged) obj).PropertyChanged += ItemPropertyChangedHandler;
_items.Add(obj);
OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, obj));
}
}

public virtual void Remove(T obj)
{
lock (_lock)
{
((INotifyPropertyChanged) obj).PropertyChanged -= ItemPropertyChangedHandler;
_items.Remove(obj);
OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, obj));
}
}

#region INotify Members

public event NotifyCollectionChangedEventHandler CollectionChanged;

protected virtual void OnCollectionChanged(NotifyCollectionChangedEventArgs args)
{
var copy = CollectionChanged;
if(copy != null)
{
CollectionChanged(this, args);
}
}

private void ItemPropertyChangedHandler(object send

Solution

Your Items initialization I believe isn't threadsafe. Use Lazy for thread safe lazy initialization instead.

The ReadOnlyCollection isn't thread safe.

A ReadOnlyCollection can support
multiple readers concurrently, as long
as the collection is not modified.
Even so, enumerating through a
collection is intrinsically not a
thread-safe procedure. To guarantee
thread safety during enumeration, you
can lock the collection during the
entire enumeration. To allow the
collection to be accessed by multiple
threads for reading and writing, you
must implement your own
synchronization.

Consider using a ReaderWriterLock to support a single writer, but multiple readers.

Furthermore, consider reading this reply, and the blog post mentioned there, which discusses why thread safe collections are so hard, and not necessarily useful.

Context

StackExchange Code Review Q#1349, answer score: 6

Revisions (0)

No revisions yet.