8
\$\begingroup\$

My version of implementation ObservableDictionary. Its should work as ObservableCollection, I hope. Based on referencesource Dictionary implementation.

.NET 4.5 framework

using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.ComponentModel;
using System.Linq;

namespace Roing.CommonUI.Collections
{
    public class ObservableDictionary<TKey, TValue> : IDictionary<TKey, TValue>, IReadOnlyDictionary<TKey, TValue>, INotifyCollectionChanged, INotifyPropertyChanged
    {
        protected IDictionary<TKey, TValue> Dictionary { get; }

        #region Constants (standart constants for collection/dictionary)

        private const string CountString = "Count";
        private const string IndexerName = "Item[]";
        private const string KeysName = "Keys";
        private const string ValuesName = "Values";

        #endregion

        #region .ctor

        public ObservableDictionary()
        {
            Dictionary = new Dictionary<TKey, TValue>();
        }

        public ObservableDictionary(IDictionary<TKey, TValue> dictionary)
        {
            Dictionary = new Dictionary<TKey, TValue>(dictionary);
        }

        public ObservableDictionary(IEqualityComparer<TKey> comparer)
        {
            Dictionary = new Dictionary<TKey, TValue>(comparer);
        }

        public ObservableDictionary(IDictionary<TKey, TValue> dictionary, IEqualityComparer<TKey> comparer)
        {
            Dictionary = new Dictionary<TKey, TValue>(dictionary,comparer);
        }

        public ObservableDictionary(int capacity, IEqualityComparer<TKey> comparer)
        {
            Dictionary = new Dictionary<TKey, TValue>(capacity, comparer);
        }

        public ObservableDictionary(int capacity)
        {
            Dictionary = new Dictionary<TKey, TValue>(capacity);
        }

        #endregion

        #region INotifyCollectionChanged and INotifyPropertyChanged

        public event NotifyCollectionChangedEventHandler CollectionChanged;
        public event PropertyChangedEventHandler PropertyChanged;

        #endregion

        #region IDictionary<TKey, TValue> Implementation

        public TValue this[TKey key]
        {
            get
            {
                return Dictionary[key];
            }
            set
            {
                InsertObject(
                    key : key, 
                    value : value,
                    appendMode :  AppendMode.Replace, 
                    oldValue : out var oldItem);

                if (oldItem != null)
                {
                    OnCollectionChanged(
                        action: NotifyCollectionChangedAction.Replace,
                        newItem: new KeyValuePair<TKey, TValue>(key, value),
                        oldItem: new KeyValuePair<TKey, TValue>(key, oldItem));
                }
                else
                {
                    OnCollectionChanged(
                        action: NotifyCollectionChangedAction.Add,
                        changedItem: new KeyValuePair<TKey, TValue>(key, value));
                }
            }
        }

        public ICollection<TKey> Keys => Dictionary.Keys;

        public ICollection<TValue> Values => Dictionary.Values;

        public int Count => Dictionary.Count;

        public bool IsReadOnly => Dictionary.IsReadOnly;

        public void Add(TKey key, TValue value)
        {
            InsertObject(
                key: key,
                value: value,
                appendMode: AppendMode.Add);

            OnCollectionChanged(
                action: NotifyCollectionChangedAction.Add,
                changedItem: new KeyValuePair<TKey, TValue>(key, value));
        }

        public void Add(KeyValuePair<TKey, TValue> item)
        {
            InsertObject(
                key: item.Key,
                value: item.Value,
                appendMode: AppendMode.Add);

            OnCollectionChanged(
                action: NotifyCollectionChangedAction.Add,
                changedItem: new KeyValuePair<TKey, TValue>(item.Key, item.Value));
        }

        public void Clear()
        {
            if (!Dictionary.Any())
            {
                return;
            }

            var removedItems = new List<KeyValuePair<TKey,TValue>>(Dictionary.ToList());
            Dictionary.Clear();
            OnCollectionChanged(
                action: NotifyCollectionChangedAction.Reset,
                newItems: null,
                oldItems: removedItems);
        }

        public bool Contains(KeyValuePair<TKey, TValue> item)
        {
            return Dictionary.Contains(item);
        }

        public bool ContainsKey(TKey key)
        {
            return Dictionary.ContainsKey(key);
        }

        public void CopyTo(KeyValuePair<TKey, TValue>[] array, int arrayIndex)
        {
            Dictionary.CopyTo(
                array: array,
                arrayIndex: arrayIndex);
        }

        public IEnumerator<KeyValuePair<TKey, TValue>> GetEnumerator()
        {
            return Dictionary.GetEnumerator();
        }

        public bool Remove(TKey key)
        {
            if(Dictionary.TryGetValue(key, out var value))
            {
                Dictionary.Remove(key);
                OnCollectionChanged(
                    action: NotifyCollectionChangedAction.Remove,
                    changedItem: new KeyValuePair<TKey, TValue>(key,value));
                return true;
            }

            return false;
        }

        public bool Remove(KeyValuePair<TKey, TValue> item)
        {
            if (Dictionary.Remove(item))
            {
                OnCollectionChanged(
                    action: NotifyCollectionChangedAction.Remove,
                    changedItem: item);
                return true;
            }
            return false;
        }

        public bool TryGetValue(TKey key, out TValue value)
        {
            return Dictionary.TryGetValue(key, out value);
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return Dictionary.GetEnumerator();
        }

        #endregion

        #region IReadOnlyDictionary

        IEnumerable<TKey> IReadOnlyDictionary<TKey, TValue>.Keys => Dictionary.Keys;
        IEnumerable<TValue> IReadOnlyDictionary<TKey, TValue>.Values => Dictionary.Values;

        #endregion

        #region ObservableDictionary inner methods

        private void InsertObject(TKey key, TValue value, AppendMode appendMode)
        {
            InsertObject(key, value, appendMode, out var trash);
        }

        private void InsertObject(TKey key, TValue value, AppendMode appendMode, out TValue oldValue)
        {
            oldValue = default(TValue);

            if (key == null)
            {
                throw new ArgumentNullException(nameof(key));
            }

            if(Dictionary.TryGetValue(key, out var item))
            {
                if(appendMode == AppendMode.Add)
                {
                    throw new ArgumentException("Item with the same key has already been added");
                }

                if (Equals(item, value))
                {
                    return;
                }

                Dictionary[key] = value;
                oldValue = item;
            }
            else
            {
                Dictionary[key] = value;
            }
        }

        private void OnPropertyChanged()
        {
            OnPropertyChanged(CountString);
            OnPropertyChanged(IndexerName);
            OnPropertyChanged(KeysName);
            OnPropertyChanged(ValuesName);
        }

        private void OnPropertyChanged(string propertyName)
        {
            if (string.IsNullOrWhiteSpace(propertyName))
            {
                OnPropertyChanged();
            }

            var handler = PropertyChanged;
            handler?.Invoke(this, new PropertyChangedEventArgs(propertyName));
        }

        private void OnCollectionChanged()
        {
            OnPropertyChanged();
            var handler = CollectionChanged;
            handler?.Invoke(
                this, new NotifyCollectionChangedEventArgs(
                    action:NotifyCollectionChangedAction.Reset));
        }

        private void OnCollectionChanged(NotifyCollectionChangedAction action, KeyValuePair<TKey, TValue> changedItem)
        {
            OnPropertyChanged();
            var handler = CollectionChanged;
            handler?.Invoke(
                this, new NotifyCollectionChangedEventArgs(
                    action:action,
                    changedItem: changedItem));
        }

        private void OnCollectionChanged(NotifyCollectionChangedAction action, KeyValuePair<TKey, TValue> newItem, KeyValuePair<TKey, TValue> oldItem)
        {
            OnPropertyChanged();
            var handler = CollectionChanged;
            handler?.Invoke(
                this, new NotifyCollectionChangedEventArgs(
                    action: action,
                    newItem: newItem,
                    oldItem: oldItem));
        }

        private void OnCollectionChanged(NotifyCollectionChangedAction action, IList newItems)
        {
            OnPropertyChanged();
            var handler = CollectionChanged;
            handler?.Invoke(
                this, new NotifyCollectionChangedEventArgs(
                    action: action,
                    changedItems: newItems));
        }

        private void OnCollectionChanged(NotifyCollectionChangedAction action, IList newItems, IList oldItems)
        {
            OnPropertyChanged();
            var handler = CollectionChanged;
            handler?.Invoke(
                this, new NotifyCollectionChangedEventArgs(
                    action: action,
                    newItems: newItems,
                    oldItems: oldItems));
        }

        #endregion

        internal enum AppendMode
        {
            Add,
            Replace
        }
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ Do you mean for c[1] = 0; c[1] = 0; to cause two CollectionChanged events? \$\endgroup\$
    – cHao
    Aug 28, 2018 at 16:36

2 Answers 2

9
\$\begingroup\$
        protected IDictionary<TKey, TValue> Dictionary { get; }

I don't find this to be a very helpful name. FWIW my default choice for something like this would be Wrapped.


        private const string CountString = "Count";
        private const string IndexerName = "Item[]";
        private const string KeysName = "Keys";
        private const string ValuesName = "Values";

Why not nameof (except for IndexerName, obviously)?


IMO there are two useful constructors missing: ObservableDictionary(IEnumerable<KeyValuePair<TKey, TValue>) and similarly with a comparer. If you've left them out because YAGNI then fair enough, but in that case I'm surprised at how many constructors were necessary.


                InsertObject(
                    key : key, 
                    value : value,
                    appendMode :  AppendMode.Replace, 
                    oldValue : out var oldItem);

                if (oldItem != null)

Hmm. Technically this isn't quite correct: after dict[foo] = null; a call to dict[foo] = bar; should be considered a replacement rather than an addition. That's why TryGetValue returns a bool and has an out-parameter for the value, rather than just returning the value and letting you test whether it's null. I suggest reworking so that InsertObject returns a bool.


        public ICollection<TKey> Keys => Dictionary.Keys;

        public ICollection<TValue> Values => Dictionary.Values;

My biggest concern with this implementation: shouldn't these two properties also be observable? I would rather bind an Items property to Keys than bind it to the dictionary with an IValueConverter to select the key.


        public void Add(TKey key, TValue value)
        {
            InsertObject(
                key: key,
                value: value,
                appendMode: AppendMode.Add);

            OnCollectionChanged(
                action: NotifyCollectionChangedAction.Add,
                changedItem: new KeyValuePair<TKey, TValue>(key, value));
        }

        public void Add(KeyValuePair<TKey, TValue> item)
        {
            InsertObject(
                key: item.Key,
                value: item.Value,
                appendMode: AppendMode.Add);

            OnCollectionChanged(
                action: NotifyCollectionChangedAction.Add,
                changedItem: new KeyValuePair<TKey, TValue>(item.Key, item.Value));
        }

DRY: one of these methods should call the other one.


            var removedItems = new List<KeyValuePair<TKey,TValue>>(Dictionary.ToList());

Either use new List<...>(Dictionary) or use Dictionary.ToList(): you don't need both. (I prefer ToList() because it saves writing out the full type).


        public bool Contains(KeyValuePair<TKey, TValue> item)
        {
            return Dictionary.Contains(item);
        }

        public bool ContainsKey(TKey key)
        {
            return Dictionary.ContainsKey(key);
        }

Does your style guide allow => notation for properties but not methods?


        private void OnCollectionChanged(NotifyCollectionChangedAction action, KeyValuePair<TKey, TValue> changedItem)
        {
            OnPropertyChanged();
            var handler = CollectionChanged;
            handler?.Invoke(
                this, new NotifyCollectionChangedEventArgs(
                    action:action,
                    changedItem: changedItem));
        }

What about the index of the changed item? I know it's a pain in the neck to implement, but it ensures maximum compatibility. If YAGNI, document that.


Finally, overall the code looks quite good. It uses modern syntactic sugar, and for the most part is well decomposed.

\$\endgroup\$
4
\$\begingroup\$

Not bad overall, but there are a few problems:

  • Clear throws an ArgumentException, because Reset events cannot reference old items.
  • this[TKey key].set checks if oldItem isn't null. That only works for reference types. For value types this will only raise Replace events instead of Add events.
  • I wouldn't expect replacing a value to trigger property change events for Count and Keys.

A few more notes:

  • I would use nameof instead of constants where possible. Also, I find that creating constants for things that are only used in one place makes code a little harder to understand without providing any maintenance benefits.
  • Why does OnPropertyChanged(string propertyName) call OnPropertyChanged() if the given property name is empty or null? It's not used in practice (so it should just be removed) but it does introduce a risk for infinite recursion (during later code modifications).
  • C# 7.0 added support for 'discards', using the special variable name _, so you can use that instead of trash.
  • There are a few unused overloads of OnCollectionChanged that can be removed.
  • The various OnCollectionChanged methods could be given more descriptive names, such as OnCollectionReset, OnValueReplaced, and so on.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.