Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Added Range Manipulation APIs to Collection<T> and ObservableCollection<T>#23018

Merged
safern merged 17 commits intodotnet:masterfrom
HoeflingSoftware:observablecollection_addrange
Mar 8, 2019
Merged

Added Range Manipulation APIs to Collection<T> and ObservableCollection<T>#23018
safern merged 17 commits intodotnet:masterfrom
HoeflingSoftware:observablecollection_addrange

Conversation

@SkyeHoefling
Copy link

@SkyeHoefling SkyeHoefling commented Mar 5, 2019

Added Range Manipulation APIs to Collection and ObservableCollection.

Part of : dotnet/corefx#10752 - migrated to: dotnet/runtime#18087
Part of: dotnet/corefx#35772

Added the following new API that was approved:

    // Adds a range to the end of the collection.
    // Raises CollectionChanged (NotifyCollectionChangedAction.Add)
    public void AddRange(IEnumerable<T> collection) => InsertItemsRange(0, collection);

    // Inserts a range
    // Raises CollectionChanged (NotifyCollectionChangedAction.Add)
    public void InsertRange(int index, IEnumerable<T> collection) => InsertItemsRange(index, collection);

    // Removes a range.
    // Raises CollectionChanged (NotifyCollectionChangedAction.Remove)
    public void RemoveRange(int index, int count) => RemoveItemsRange(index, count);

    // Will allow to replace a range with fewer, equal, or more items.
    // Raises CollectionChanged (NotifyCollectionChangedAction.Replace)
    public void ReplaceRange(int index, int count, IEnumerable<T> collection)
    {
         RemoveItemsRange(index, count);
         InsertItemsRange(index, collection);
    }

    #region virtual methods
    protected virtual void InsertItemsRange(int index, IEnumerable<T> collection);
    protected virtual void RemoveItemsRange(int index, int count);
    #endregion

Looking forward to everyone's feedback on this

…p to ObservableCollection<T>. AddRange, InsertRange, RemoveRange and Replace Range
@ahsonkhan
Copy link

Based on https://github.com/dotnet/corefx/issues/10752#issuecomment-428312248, you are missing ReplaceItemsRange

@SkyeHoefling
Copy link
Author

Based on dotnet/corefx#10752 (comment), you are missing ReplaceItemsRange

Thanks for pointing this out, I was going off the update at the top and missed this API. I'll get this added in the next update and include tests in the associated PR for CoreFX

Andrew Hoefling added 5 commits March 5, 2019 18:14
… because the array changes with each iteration of the for loop and will cause side-effects which may include index out of range exceptions. This needs to be index because as the array shrinks the index is always at the correct position.
…n checks that happen using the RemoveAt API. RemoveItem gives us direct access to invoke the command
…ance improvements. If the underlying `items` is using List<T> we should use it's InsertRange method since it is optimized, othersie we use InsertItem
…(index + count) > items.Count and if it is true throw ArgumentException
Copy link

@ahsonkhan ahsonkhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some leftover comments. Otherwise, LGTM.

Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once @ahsonkhan's last comments are solved.

@safern safern merged commit d29c78b into dotnet:master Mar 8, 2019
@safern
Copy link
Member

safern commented Mar 8, 2019

Thanks @ahoefling now we can move to corefx's PR once CoreCLR flows there 😄

@jkotas
Copy link
Member

jkotas commented Mar 8, 2019

@safern Try to hit squash & merge next time ... I know it is easy to press the wrong button by accident.

@safern
Copy link
Member

safern commented Mar 8, 2019

@safern Try to hit squash & merge next time ... I know it is easy to press the wrong button by accident.

Yeah I just realized I rebased and merge. Sorry about that, my default was "Squash and merge" I don't know how it was changed 😭

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants