Simple ObservableDictionary implimentation

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
7
down vote

favorite












My version of implimentation 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 .ctr

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









share|improve this question




















  • Do you mean for c[1] = 0; c[1] = 0; to cause two CollectionChanged events?
    – cHao
    Aug 28 at 16:36

















up vote
7
down vote

favorite












My version of implimentation 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 .ctr

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









share|improve this question




















  • Do you mean for c[1] = 0; c[1] = 0; to cause two CollectionChanged events?
    – cHao
    Aug 28 at 16:36













up vote
7
down vote

favorite









up vote
7
down vote

favorite











My version of implimentation 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 .ctr

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









share|improve this question












My version of implimentation 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 .ctr

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











share|improve this question











share|improve this question




share|improve this question










asked Aug 28 at 12:41









ParanoidPanda

1704




1704











  • Do you mean for c[1] = 0; c[1] = 0; to cause two CollectionChanged events?
    – cHao
    Aug 28 at 16:36

















  • Do you mean for c[1] = 0; c[1] = 0; to cause two CollectionChanged events?
    – cHao
    Aug 28 at 16:36
















Do you mean for c[1] = 0; c[1] = 0; to cause two CollectionChanged events?
– cHao
Aug 28 at 16:36





Do you mean for c[1] = 0; c[1] = 0; to cause two CollectionChanged events?
– cHao
Aug 28 at 16:36











2 Answers
2






active

oldest

votes

















up vote
9
down vote



accepted











 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.






share|improve this answer



























    up vote
    4
    down vote













    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.





    share|improve this answer




















      Your Answer




      StackExchange.ifUsing("editor", function ()
      return StackExchange.using("mathjaxEditing", function ()
      StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
      StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
      );
      );
      , "mathjax-editing");

      StackExchange.ifUsing("editor", function ()
      StackExchange.using("externalEditor", function ()
      StackExchange.using("snippets", function ()
      StackExchange.snippets.init();
      );
      );
      , "code-snippets");

      StackExchange.ready(function()
      var channelOptions =
      tags: "".split(" "),
      id: "196"
      ;
      initTagRenderer("".split(" "), "".split(" "), channelOptions);

      StackExchange.using("externalEditor", function()
      // Have to fire editor after snippets, if snippets enabled
      if (StackExchange.settings.snippets.snippetsEnabled)
      StackExchange.using("snippets", function()
      createEditor();
      );

      else
      createEditor();

      );

      function createEditor()
      StackExchange.prepareEditor(
      heartbeatType: 'answer',
      convertImagesToLinks: false,
      noModals: false,
      showLowRepImageUploadWarning: true,
      reputationToPostImages: null,
      bindNavPrevention: true,
      postfix: "",
      onDemand: true,
      discardSelector: ".discard-answer"
      ,immediatelyShowMarkdownHelp:true
      );



      );













       

      draft saved


      draft discarded


















      StackExchange.ready(
      function ()
      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f202663%2fsimple-observabledictionary-implimentation%23new-answer', 'question_page');

      );

      Post as a guest






























      2 Answers
      2






      active

      oldest

      votes








      2 Answers
      2






      active

      oldest

      votes









      active

      oldest

      votes






      active

      oldest

      votes








      up vote
      9
      down vote



      accepted











       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.






      share|improve this answer
























        up vote
        9
        down vote



        accepted











         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.






        share|improve this answer






















          up vote
          9
          down vote



          accepted







          up vote
          9
          down vote



          accepted







           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.






          share|improve this answer













           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.







          share|improve this answer












          share|improve this answer



          share|improve this answer










          answered Aug 28 at 14:57









          Peter Taylor

          14.4k2454




          14.4k2454






















              up vote
              4
              down vote













              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.





              share|improve this answer
























                up vote
                4
                down vote













                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.





                share|improve this answer






















                  up vote
                  4
                  down vote










                  up vote
                  4
                  down vote









                  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.





                  share|improve this answer












                  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.






                  share|improve this answer












                  share|improve this answer



                  share|improve this answer










                  answered Aug 28 at 15:01









                  Pieter Witvoet

                  3,771721




                  3,771721



























                       

                      draft saved


                      draft discarded















































                       


                      draft saved


                      draft discarded














                      StackExchange.ready(
                      function ()
                      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f202663%2fsimple-observabledictionary-implimentation%23new-answer', 'question_page');

                      );

                      Post as a guest













































































                      Comments

                      Popular posts from this blog

                      What does second last employer means? [closed]

                      List of Gilmore Girls characters

                      One-line joke