Simple ObservableDictionary implimentation










7












$begingroup$


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









$endgroup$











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
















7












$begingroup$


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









$endgroup$











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














7












7








7





$begingroup$


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









$endgroup$




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









c#






share|improve this question













share|improve this question











share|improve this question




share|improve this question










asked Aug 28 '18 at 12:41









ParanoidPandaParanoidPanda

1704




1704











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

















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
















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





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











2 Answers
2






active

oldest

votes


















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.






share|improve this answer









$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.





    share|improve this answer









    $endgroup$













      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',
      autoActivateHeartbeat: false,
      convertImagesToLinks: false,
      noModals: true,
      showLowRepImageUploadWarning: true,
      reputationToPostImages: null,
      bindNavPrevention: true,
      postfix: "",
      imageUploader:
      brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
      contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
      allowUrls: true
      ,
      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















      Required, but never shown

























      2 Answers
      2






      active

      oldest

      votes








      2 Answers
      2






      active

      oldest

      votes









      active

      oldest

      votes






      active

      oldest

      votes









      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.






      share|improve this answer









      $endgroup$

















        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.






        share|improve this answer









        $endgroup$















          9












          9








          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.






          share|improve this answer









          $endgroup$




           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 '18 at 14:57









          Peter TaylorPeter Taylor

          18.2k2963




          18.2k2963























              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.





              share|improve this answer









              $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.





                share|improve this answer









                $endgroup$















                  4












                  4








                  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.





                  share|improve this answer









                  $endgroup$



                  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 '18 at 15:01









                  Pieter WitvoetPieter Witvoet

                  6,444826




                  6,444826



























                      draft saved

                      draft discarded
















































                      Thanks for contributing an answer to Code Review Stack Exchange!


                      • Please be sure to answer the question. Provide details and share your research!

                      But avoid


                      • Asking for help, clarification, or responding to other answers.

                      • Making statements based on opinion; back them up with references or personal experience.

                      Use MathJax to format equations. MathJax reference.


                      To learn more, see our tips on writing great answers.




                      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















                      Required, but never shown





















































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown

































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown







                      Popular posts from this blog

                      𛂒𛀶,𛀽𛀑𛂀𛃧𛂓𛀙𛃆𛃑𛃷𛂟𛁡𛀢𛀟𛁤𛂽𛁕𛁪𛂟𛂯,𛁞𛂧𛀴𛁄𛁠𛁼𛂿𛀤 𛂘,𛁺𛂾𛃭𛃭𛃵𛀺,𛂣𛃍𛂖𛃶 𛀸𛃀𛂖𛁶𛁏𛁚 𛂢𛂞 𛁰𛂆𛀔,𛁸𛀽𛁓𛃋𛂇𛃧𛀧𛃣𛂐𛃇,𛂂𛃻𛃲𛁬𛃞𛀧𛃃𛀅 𛂭𛁠𛁡𛃇𛀷𛃓𛁥,𛁙𛁘𛁞𛃸𛁸𛃣𛁜,𛂛,𛃿,𛁯𛂘𛂌𛃛𛁱𛃌𛂈𛂇 𛁊𛃲,𛀕𛃴𛀜 𛀶𛂆𛀶𛃟𛂉𛀣,𛂐𛁞𛁾 𛁷𛂑𛁳𛂯𛀬𛃅,𛃶𛁼

                      Edmonton

                      Crossroads (UK TV series)