Skip to content

Commit

Permalink
#1691 FastObservableDictionary should allow null-values and not throw…
Browse files Browse the repository at this point in the history 
… ArgumentNullException
  • Loading branch information
@GeertvanHorrik
GeertvanHorrik committed Nov 20, 2020
1 parent f7e6883 commit 0d03dd8
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 10 deletions.
1 change: 1 addition & 0 deletions doc/history.txt
View file
Expand Up @@ -30,6 +30,7 @@ Release date
Added/fixed
===========
(+) #1713 Add support for .NET 5.0
(*) #1691 FastObservableDictionary should allow null-values and not throw ArgumentNullException
(*) #1714 Update all system packages to 5.0


Expand Down
32 changes: 25 additions & 7 deletions src/Catel.MVVM/Collections/FastObservableDictionary.cs
View file
Expand Up @@ -64,8 +64,6 @@ public class FastObservableDictionary<TKey, TValue> : IDictionary<TKey, TValue>,
/// <value><c>true</c> if events should automatically be dispatched to the UI thread; otherwise, <c>false</c>.</value>
public bool AutomaticallyDispatchChangeNotifications { get; set; } = true;



/// <see cref="Dictionary{TKey,TValue}.Comparer"/>>
public IEqualityComparer<TKey> Comparer => _dict.Comparer;
#endregion
Expand All @@ -77,12 +75,14 @@ public FastObservableDictionary()
_dictIndexMapping = new Dictionary<TKey, int>();
_list = new List<TKey>();
}

public FastObservableDictionary(int capacity)
{
_dict = new Dictionary<TKey, TValue>(capacity);
_dictIndexMapping = new Dictionary<TKey, int>(capacity);
_list = new List<TKey>(capacity);
}

public FastObservableDictionary(IEnumerable<KeyValuePair<TKey, TValue>> originalDict)
{
if (originalDict is ICollection<KeyValuePair<TKey, TValue>> collection)
Expand All @@ -93,21 +93,25 @@ public FastObservableDictionary(IEnumerable<KeyValuePair<TKey, TValue>> original
}
InsertMultipleValues(0, originalDict, false);
}

public FastObservableDictionary(IEqualityComparer<TKey> comparer)
{
_list = new List<TKey>();
_dictIndexMapping = new Dictionary<TKey, int>(comparer);
_dict = new Dictionary<TKey, TValue>(comparer);
}

public FastObservableDictionary(IDictionary<TKey, TValue> dictionary) : this((IEnumerable<KeyValuePair<TKey, TValue>>)dictionary)
{
}

public FastObservableDictionary(int capacity, IEqualityComparer<TKey> comparer)
{
_list = new List<TKey>(capacity);
_dictIndexMapping = new Dictionary<TKey, int>(capacity, comparer);
_dict = new Dictionary<TKey, TValue>(capacity, comparer);
}

public FastObservableDictionary(IDictionary<TKey, TValue> dictionary, IEqualityComparer<TKey> comparer)
{
Argument.IsNotNull(nameof(dictionary), dictionary);
Expand Down Expand Up @@ -200,7 +204,7 @@ private void WriteRemoveSuspension(KeyValuePair<TKey, TValue> changedItem, int c
public virtual void InsertSingleValue(TKey key, TValue newValue, bool checkKeyDuplication)
{
Argument.IsNotNull(nameof(key), key);
Argument.IsNotNull(nameof(newValue), newValue);

var changedItem = new KeyValuePair<TKey, TValue>(key, newValue);
int changedIndex;
if (checkKeyDuplication && _dictIndexMapping.TryGetValue(key, out var oldIndex) && _dict.TryGetValue(key, out var oldValue))
Expand Down Expand Up @@ -267,12 +271,13 @@ public virtual void InsertSingleValue(TKey key, TValue newValue, bool checkKeyDu
public virtual void InsertSingleValue(int index, TKey key, TValue newValue, bool checkKeyDuplication)
{
Argument.IsNotNull(nameof(key), key);
Argument.IsNotNull(nameof(newValue), newValue);

// Check
if (_suspensionContext != null && _suspensionContext.Mode == SuspensionMode.Removing)
{
throw Log.ErrorAndCreateException<InvalidOperationException>("Adding items is not allowed in mode SuspensionMode.Removing.");
}

var changedItem = new KeyValuePair<TKey, TValue>(key, newValue);
int changedIndex = index;
if (checkKeyDuplication && _dict.TryGetValue(key, out var oldValue) && _dictIndexMapping.TryGetValue(key, out var oldIndex))
Expand Down Expand Up @@ -340,21 +345,21 @@ public virtual void MoveItem(int oldIndex, int newIndex)
protected virtual void InternalMoveItem(int oldIndex, int newIndex, TKey key, TValue element)
{
Argument.IsNotNull(nameof(key), key);
Argument.IsNotNull(nameof(element), element);

// Check
if (_suspensionContext != null && (_suspensionContext.Mode != SuspensionMode.None && _suspensionContext.Mode != SuspensionMode.Silent && !_suspensionContext.IsMixedMode()))
{
throw Log.ErrorAndCreateException<InvalidOperationException>($"Moving items is only allowed in SuspensionMode.None, SuspensionMode.Silent or mixed modes, current mode is '{_suspensionContext.Mode}'");
}


var temp = _list[oldIndex];
var sign = Math.Sign(newIndex - oldIndex);
var checkCondition = sign > 0 ? (Func<int, bool>)((int i) => i < newIndex) : ((int i) => i > newIndex);
for (var i = oldIndex; checkCondition(i); i += sign) //negative sign
{
_dictIndexMapping[_list[i] = _list[i + sign]] = i;
}

_list[newIndex] = temp;
_dictIndexMapping[temp] = newIndex;

Expand All @@ -380,6 +385,7 @@ protected virtual void InternalMoveItem(int oldIndex, int newIndex, TKey key, TV
public virtual bool TryRemoveSingleValue(TKey keyToRemove, out TValue value)
{
Argument.IsNotNull(nameof(keyToRemove), keyToRemove);

if (_dictIndexMapping.TryGetValue(keyToRemove, out var removedKeyIndex) && _dict.TryGetValue(keyToRemove, out var removedKeyValue))
{
// Check
Expand Down Expand Up @@ -464,6 +470,7 @@ public virtual void InsertMultipleValues(IEnumerable<KeyValuePair<TKey, TValue>>
{
newValues = newValues.Where(x => !_dict.ContainsKey(x.Key));
}

var startIndex = _list.Count;
if (newValues is ICollection<KeyValuePair<TKey, TValue>> collection)
{
Expand Down Expand Up @@ -543,6 +550,7 @@ public virtual void InsertMultipleValues(int startIndex, IEnumerable<KeyValuePai
{
newValues = newValues.Where(x => !_dict.ContainsKey(x.Key));
}

var counterIndex = startIndex;
if (newValues is ICollection<KeyValuePair<TKey, TValue>> collection)
{
Expand Down Expand Up @@ -616,6 +624,7 @@ public virtual void RemoveMultipleValues(IEnumerable<TKey> keysToRemove)
Argument.IsNotNull(nameof(keysToRemove), keysToRemove);

Dictionary<int, List<KeyValuePair<TKey, TValue>>> removedList;

if (keysToRemove is ICollection<TKey> collectionOfKeysToRemove)
{
removedList = new Dictionary<int, List<KeyValuePair<TKey, TValue>>>(collectionOfKeysToRemove.Count);
Expand All @@ -629,9 +638,13 @@ public virtual void RemoveMultipleValues(IEnumerable<TKey> keysToRemove)
if (_dictIndexMapping.TryGetValue(keyToRemove, out var removedKeyIndex) && _dict.TryGetValue(keyToRemove, out var valueToRemove))
{
if (removedList.TryGetValue(removedKeyIndex - 1, out var toRemoveList))
{
toRemoveList.Add(new KeyValuePair<TKey, TValue>(keyToRemove, valueToRemove));
}
else
{
removedList[removedKeyIndex] = new List<KeyValuePair<TKey, TValue>>() { new KeyValuePair<TKey, TValue>(keyToRemove, valueToRemove) };
}

_dict.Remove(keyToRemove);
_dictIndexMapping.Remove(keyToRemove);
Expand All @@ -642,6 +655,7 @@ public virtual void RemoveMultipleValues(IEnumerable<TKey> keysToRemove)
OnPropertyChanged(_cachedCountArgs);
OnPropertyChanged(_cachedKeysArgs);
OnPropertyChanged(_cachedValuesArgs);

foreach (var range in removedList)
{
_list.RemoveRange(range.Key, range.Value.Count);
Expand All @@ -658,6 +672,7 @@ public virtual void RemoveAllItems()
{
throw Log.ErrorAndCreateException<InvalidOperationException>($"Clearing items is only allowed in SuspensionMode.None, SuspensionMode.Silent or mixed modes, current mode is '{_suspensionContext.Mode}'");
}

List<KeyValuePair<TKey, TValue>> copyList = null;
if (_suspensionContext != null && _suspensionContext.IsMixedMode())
{
Expand Down Expand Up @@ -690,6 +705,7 @@ public virtual void RemoveAllItems()
public void Add(KeyValuePair<TKey, TValue> item)
{
Argument.IsNotNull(nameof(item), item);

InsertSingleValue(item.Key, item.Value, true);
}

Expand All @@ -708,6 +724,7 @@ public bool Contains(KeyValuePair<TKey, TValue> item)
public void CopyTo(KeyValuePair<TKey, TValue>[] array, int arrayIndex)
{
Argument.IsNotNull(nameof(array), array);

if (array.Length - arrayIndex < Count) throw new IndexOutOfRangeException("Array doesn't have enough space to copy all the elements");
for (var i = 0; i < Count; i++)
{
Expand All @@ -720,6 +737,7 @@ public void CopyTo(KeyValuePair<TKey, TValue>[] array, int arrayIndex)
public bool Remove(KeyValuePair<TKey, TValue> item)
{
Argument.IsNotNull(nameof(item), item);

return TryRemoveSingleValue(item.Key, out _);
}
#endregion
Expand Down Expand Up @@ -818,7 +836,7 @@ public bool Contains(object key)
public void Add(object key, object value)
{
Argument.IsNotNull(nameof(key), key);
Argument.IsNotNull(nameof(value), value);

if (key is TKey castedKey)
{
if (value is TValue castedValue)
Expand Down
16 changes: 13 additions & 3 deletions src/Catel.Tests/Collections/FastObservableDictionaryFacts.cs
View file
Expand Up @@ -67,11 +67,21 @@ public void ThrowsInvalidCastExceptionForAddObject()
}

[Test]
public void ThrowsArgumentNullExceptionForAddObject()
public void ThrowsArgumentNullExceptionForNullKey()
{
var observableDictionary = new FastObservableDictionary<int, int>();
var observableDictionary = new FastObservableDictionary<object, int>();

Assert.That(() => observableDictionary.Add(null, 1), Throws.ArgumentNullException);
}

[Test]
public void AllowsNullValues()
{
var observableDictionary = new FastObservableDictionary<int, object>();

observableDictionary.Add(1, null);

Assert.That(() => observableDictionary.Add(1, null), Throws.ArgumentNullException);
Assert.IsNull(observableDictionary[1]);
}

[Test]
Expand Down

0 comments on commit 0d03dd8

Please sign in to comment.