Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Commit

Permalink
Added Collection<T> AddRange and InsertRange and added new XUnit test…
Browse files Browse the repository at this point in the history
…s to cover new APIs

Added missing APIs from Collection<T>; Added new unit tests for RemoveRange, ReplaceRange, InsertItemsRange and RemoveItemsRange

Updated AddRange in Collection<T> to add items to the end of the collection

Added CollectionChanged tests for ObservableCollection<T> for InsertRange, AddRange, RemoveRange and ReplaceRange

Removing API changes as these will be mirrored from CoreCLR

Updated array assertions to use Span<T> to simplify logic

Sorted order of new API methods

Simplified CollectionTests Assertion to use Span<T> instead of calling out each item in the array

Added missing API ReplaceItemsRange and updated unit tests for check for overflow errors on RemoveItems

Added overrides for ObservableCollection<T> ItemsRange API to only fire CollectionChanged once per API call. Before this was firing for each item in the collection which is not the desired result. Updated unit tests to verify this logic

Updated RemoveItemsRange to prevent int.MaxValue overflow errors

Added int.MaxValue overflow tests for Collection<T> and ObservableCollection<T> when invoking RemoveItemsRange API

Added additional RemoveRange unit tests to cover when Count is Less than 0 or when Count is 0

Updated RemoveRange overflow tests to check for ArgumentException for new business rules

Updated ObservableCollection overflow test to assert ArgumentException

Added try-finally block to ObservableCollection<T> to certify that _skipCollectionChanged is always set to false after a collection manipulation in case an exception is thrown in a subclass. Added new test class to test the new rules on the example of a NonNullObservableCollection<T>

CollectionChanged events do not fire on RemoveRange if the count == 0

Updated ObservableCollection<T> to only throw the 3 required events when ReplaceItemsRange is invoked. Updated OnChangedEvent methods to check for IList and cast as necessary. Added additional exception handling to reset _skipRaisingEvents bool. Added unit tests to verify event and event parameters are being passed correctly.

Added Collection<T> AddRange and InsertRange and added new XUnit tests to cover new APIs

Simplified CollectionTests Assertion to use Span<T> instead of calling out each item in the array

Added additional RemoveRange unit tests to cover when Count is Less than 0 or when Count is 0

Updated ObservableCollection<T> to only throw the 3 required events when ReplaceItemsRange is invoked. Updated OnChangedEvent methods to check for IList and cast as necessary. Added additional exception handling to reset _skipRaisingEvents bool. Added unit tests to verify event and event parameters are being passed correctly.

Updated ReplaceRange event to pass in the OldItems and NewItems to match the event docs

Simplified CollectionChanged 'NewItems for InsertItemsRange

Updated removedItems to use an array instead of a List<T>

Added new OnCollectionChanged overloads to reduce need for additional if checks.

Removed stale code

Optimized ReplaceItemsRange by simplifying the itemsToReplace which is no an array instead of a List<T>

Removed (IList) cast in ReplaceItemsRange when raising CollectionChanged

Removed temp array in RemoveRange which decreases memory allocation needed
  • Loading branch information
Andrew Hoefling committed Mar 16, 2019
1 parent decd797 commit 70da65f
Show file tree
Hide file tree
Showing 6 changed files with 950 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ public class ObservableCollection<T> : Collection<T>, INotifyCollectionChanged,
[NonSerialized]
private int _blockReentrancyCount;

[NonSerialized]
private bool _skipRaisingEvents;

/// <summary>
/// Initializes a new instance of ObservableCollection that is empty and has default initial capacity.
/// </summary>
Expand Down Expand Up @@ -121,11 +124,92 @@ protected override void RemoveItem(int index)

base.RemoveItem(index);

if (!_skipRaisingEvents)
{
OnCountPropertyChanged();
OnIndexerPropertyChanged();
OnCollectionChanged(NotifyCollectionChangedAction.Remove, removedItem, index);
}
}

/// <summary>
/// Called by base class Collection&lt;T&gt; when a count of items is removed from the list;
/// raises a CollectionChanged event to any listeners.
/// </summary>
protected override void RemoveItemsRange(int index, int count)
{
CheckReentrancy();

T[] removedItems = null;

bool ignore = _skipRaisingEvents;
if (!ignore)
{
_skipRaisingEvents = true;

if (count > 0)
{
removedItems = new T[count];
for (int i = 0; i < count; i++)
{
removedItems[i] = this[index + i];
}
}
}

try
{
base.RemoveItemsRange(index, count);
}
finally
{
if (!ignore)
{
_skipRaisingEvents = false;
}
}

if (count > 0 && !_skipRaisingEvents)
{
OnCountPropertyChanged();
OnIndexerPropertyChanged();
OnCollectionChanged(NotifyCollectionChangedAction.Remove, removedItems, index);
}
}

/// <summary>
/// Called by base class Collection&lt;T&gt; when a collection of items is added to list;
/// raises a CollectionChanged event to any listeners.
/// </summary>
protected override void ReplaceItemsRange(int index, int count, IEnumerable<T> collection)
{
CheckReentrancy();

_skipRaisingEvents = true;

T[] itemsToReplace = new T[count - index];
for (int i = index; i < count; i++)
{
itemsToReplace[i] = this[i];
}

try
{
base.ReplaceItemsRange(index, count, collection);
}
finally
{
_skipRaisingEvents = false;
}

IList newItems = collection is IList list ? list : new List<T>(collection);

OnCountPropertyChanged();
OnIndexerPropertyChanged();
OnCollectionChanged(NotifyCollectionChangedAction.Remove, removedItem, index);
OnCollectionChanged(NotifyCollectionChangedAction.Replace, itemsToReplace, newItems, index);
}


/// <summary>
/// Called by base class Collection&lt;T&gt; when an item is added to list;
/// raises a CollectionChanged event to any listeners.
Expand All @@ -135,9 +219,48 @@ protected override void InsertItem(int index, T item)
CheckReentrancy();
base.InsertItem(index, item);

OnCountPropertyChanged();
OnIndexerPropertyChanged();
OnCollectionChanged(NotifyCollectionChangedAction.Add, item, index);
if (!_skipRaisingEvents)
{
OnCountPropertyChanged();
OnIndexerPropertyChanged();
OnCollectionChanged(NotifyCollectionChangedAction.Add, item, index);
}
}

/// <summary>
/// Called by base class Collection&lt;T&gt; when a collection of items is added to list;
/// raises a CollectionChanged event to any listeners.
/// </summary>
protected override void InsertItemsRange(int index, IEnumerable<T> collection)
{
CheckReentrancy();

bool ignore = _skipRaisingEvents;
if (!ignore)
{
_skipRaisingEvents = true;
}

try
{
base.InsertItemsRange(index, collection);
}
finally
{
if (!ignore)
{
_skipRaisingEvents = false;
}
}

if (!_skipRaisingEvents)
{
IList newItems = collection is IList list ? list : new List<T>(collection);

OnCountPropertyChanged();
OnIndexerPropertyChanged();
OnCollectionChanged(NotifyCollectionChangedAction.Add, newItems, index);
}
}

/// <summary>
Expand Down Expand Up @@ -265,6 +388,14 @@ private void OnCollectionChanged(NotifyCollectionChangedAction action, object it
OnCollectionChanged(new NotifyCollectionChangedEventArgs(action, item, index));
}

/// <summary>
/// Helper to raise CollectionChanged event to any listeners
/// </summary>
private void OnCollectionChanged(NotifyCollectionChangedAction action, IList items, int index)
{
OnCollectionChanged(new NotifyCollectionChangedEventArgs(action, items, index));
}

/// <summary>
/// Helper to raise CollectionChanged event to any listeners
/// </summary>
Expand All @@ -281,6 +412,14 @@ private void OnCollectionChanged(NotifyCollectionChangedAction action, object ol
OnCollectionChanged(new NotifyCollectionChangedEventArgs(action, newItem, oldItem, index));
}

/// <summary>
/// Helper to raise CollectionChanged event to any listeners
/// </summary>
private void OnCollectionChanged(NotifyCollectionChangedAction action, IList oldItems, IList newItems, int index)
{
OnCollectionChanged(new NotifyCollectionChangedEventArgs(action, newItems, oldItems, index));
}

/// <summary>
/// Helper to raise CollectionChanged event with action == Reset to any listeners
/// </summary>
Expand Down
Loading

0 comments on commit 70da65f

Please sign in to comment.