Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align API surface of immutable collections and their corresponding builder types #66550

Merged
merged 14 commits into from
May 4, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
// Changes to this file must follow the https://aka.ms/api-review process.
// ------------------------------------------------------------------------------

using System.Collections.Generic;

namespace System.Collections.Immutable
{
public partial interface IImmutableDictionary<TKey, TValue> : System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<TKey, TValue>>, System.Collections.Generic.IReadOnlyCollection<System.Collections.Generic.KeyValuePair<TKey, TValue>>, System.Collections.Generic.IReadOnlyDictionary<TKey, TValue>, System.Collections.IEnumerable
Expand Down Expand Up @@ -122,6 +124,10 @@ public static partial class ImmutableArray
public System.Collections.Immutable.ImmutableArray<T> Add(T item) { throw null; }
public System.Collections.Immutable.ImmutableArray<T> AddRange(System.Collections.Generic.IEnumerable<T> items) { throw null; }
public System.Collections.Immutable.ImmutableArray<T> AddRange(System.Collections.Immutable.ImmutableArray<T> items) { throw null; }
public System.Collections.Immutable.ImmutableArray<T> AddRange(T[] items, int length) { throw null; }
public System.Collections.Immutable.ImmutableArray<T> AddRange<TDerived>(TDerived[] items) where TDerived : T { throw null; }
public System.Collections.Immutable.ImmutableArray<T> AddRange(ImmutableArray<T> items, int length) { throw null; }
public System.Collections.Immutable.ImmutableArray<T> AddRange<TDerived>(ImmutableArray<TDerived> items) where TDerived : T { throw null; }
public System.Collections.Immutable.ImmutableArray<T> AddRange(System.ReadOnlySpan<T> items) { throw null; }
public System.Collections.Immutable.ImmutableArray<T> AddRange(params T[] items) { throw null; }
public System.ReadOnlyMemory<T> AsMemory() { throw null; }
Expand Down Expand Up @@ -244,21 +250,33 @@ public void AddRange<TDerived>(System.ReadOnlySpan<TDerived> items) where TDeriv
public void Clear() { }
public bool Contains(T item) { throw null; }
public void CopyTo(T[] array, int index) { }
public void CopyTo(T[] destination) { throw null; }
public void CopyTo(int sourceIndex, T[] destination, int destinationIndex, int length) { throw null; }
public void CopyTo(System.Span<T> destination) { }
public System.Collections.Generic.IEnumerator<T> GetEnumerator() { throw null; }
public int IndexOf(T item) { throw null; }
public int IndexOf(T item, int startIndex) { throw null; }
public int IndexOf(T item, int startIndex, int count) { throw null; }
public int IndexOf(T item, int startIndex, IEqualityComparer<T>? equalityComparer) { throw null; }
public int IndexOf(T item, int startIndex, int count, System.Collections.Generic.IEqualityComparer<T>? equalityComparer) { throw null; }
public void Insert(int index, T item) { }
public void InsertRange(int index, IEnumerable<T> items) { throw null; }
public void InsertRange(int index, ImmutableArray<T> items) { throw null; }
public ref readonly T ItemRef(int index) { throw null; }
public int LastIndexOf(T item) { throw null; }
public int LastIndexOf(T item, int startIndex) { throw null; }
public int LastIndexOf(T item, int startIndex, int count) { throw null; }
public int LastIndexOf(T item, int startIndex, int count, System.Collections.Generic.IEqualityComparer<T>? equalityComparer) { throw null; }
public System.Collections.Immutable.ImmutableArray<T> MoveToImmutable() { throw null; }
public bool Remove(T element) { throw null; }
public void Remove(T element, IEqualityComparer<T>? equalityComparer) { throw null; }
public void RemoveAll(Predicate<T> match) { throw null; }
public void RemoveAt(int index) { }
public void RemoveRange(int index, int length) { throw null; }
public void RemoveRange(IEnumerable<T> items) { throw null; }
public void RemoveRange(IEnumerable<T> items, IEqualityComparer<T>? equalityComparer) { throw null; }
public void Replace(T oldValue, T newValue) { throw null; }
public void Replace(T oldValue, T newValue, IEqualityComparer<T>? equalityComparer) { throw null; }
public void Reverse() { }
public void Sort() { }
public void Sort(System.Collections.Generic.IComparer<T>? comparer) { }
Expand Down Expand Up @@ -702,8 +720,14 @@ public void InsertRange(int index, System.Collections.Generic.IEnumerable<T> ite
public int LastIndexOf(T item, int startIndex, int count) { throw null; }
public int LastIndexOf(T item, int startIndex, int count, System.Collections.Generic.IEqualityComparer<T>? equalityComparer) { throw null; }
public bool Remove(T item) { throw null; }
public void Remove(T value, IEqualityComparer<T>? equalityComparer) { throw null; }
Copy link
Member

Choose a reason for hiding this comment

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

Similar to the above, cc @terrajobst

Suggested change
public void Remove(T value, IEqualityComparer<T>? equalityComparer) { throw null; }
public bool Remove(T item, IEqualityComparer<T>? equalityComparer) { throw null; }

Copy link
Member

Choose a reason for hiding this comment

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

@RaymondHuy could you please update the return type? It should match that of the existing overload.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @eiriktsarpalis I have updated the PR.

public int RemoveAll(System.Predicate<T> match) { throw null; }
public void RemoveAt(int index) { }
public void RemoveRange(int index, int count) { throw null; }
public void RemoveRange(IEnumerable<T> items) { throw null; }
public void RemoveRange(IEnumerable<T> items, IEqualityComparer<T>? equalityComparer) { throw null; }
public void Replace(T oldValue, T newValue) { throw null; }
public void Replace(T oldValue, T newValue, IEqualityComparer<T>? equalityComparer) { throw null; }
public void Reverse() { }
public void Reverse(int index, int count) { }
public void Sort() { }
Expand Down Expand Up @@ -1008,6 +1032,7 @@ public void Clear() { }
public void ExceptWith(System.Collections.Generic.IEnumerable<T> other) { }
public System.Collections.Immutable.ImmutableSortedSet<T>.Enumerator GetEnumerator() { throw null; }
public void IntersectWith(System.Collections.Generic.IEnumerable<T> other) { }
public int IndexOf(T item) { throw null; }
public bool IsProperSubsetOf(System.Collections.Generic.IEnumerable<T> other) { throw null; }
public bool IsProperSupersetOf(System.Collections.Generic.IEnumerable<T> other) { throw null; }
public bool IsSubsetOf(System.Collections.Generic.IEnumerable<T> other) { throw null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,61 @@ public void Insert(int index, T item)
_elements[index] = item;
}

/// <summary>
/// Inserts the specified values at the specified index.
/// </summary>
/// <param name="index">The index at which to insert the value.</param>
/// <param name="items">The elements to insert.</param>
public void InsertRange(int index, IEnumerable<T> items)
{
Requires.Range(index >= 0 && index <= this.Count, nameof(index));
Requires.NotNull(items, nameof(items));

int count = ImmutableExtensions.GetCount(ref items);
this.EnsureCapacity(this.Count + count);

if (index != this.Count)
{
Array.Copy(_elements, index, _elements, index + count, _count - index);
}

if (!items.TryCopyTo(_elements, index))
{
foreach (var item in items)
{
_elements[index++] = item;
}
}

_count += count;
}

/// <summary>
/// Inserts the specified values at the specified index.
/// </summary>
/// <param name="index">The index at which to insert the value.</param>
/// <param name="items">The elements to insert.</param>
public void InsertRange(int index, ImmutableArray<T> items)
{
Requires.Range(index >= 0 && index <= this.Count, nameof(index));

if (items.IsEmpty)
{
return;
}

this.EnsureCapacity(this.Count + items.Length);

if (index != this.Count)
{
Array.Copy(_elements, index, _elements, index + items.Length, _count - index);
}

Array.Copy(items.array!, 0, _elements, index, items.Length);

_count += items.Length;
}

/// <summary>
/// Adds an item to the <see cref="ICollection{T}"/>.
/// </summary>
Expand Down Expand Up @@ -422,6 +477,50 @@ public bool Remove(T element)
return false;
}

/// <summary>
/// Removes the specified element.
/// </summary>
/// <param name="item">The item to remove.</param>
/// <param name="equalityComparer">
/// The equality comparer to use in the search.
/// If <c>null</c>, <see cref="EqualityComparer{T}.Default"/> is used.
/// </param>
Copy link
Member

Choose a reason for hiding this comment

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

Please add a <returns /> element as above.

public void Remove(T item, IEqualityComparer<T>? equalityComparer)
{
int index = this.IndexOf(item, 0, _count, equalityComparer);

if (index >= 0)
{
this.RemoveAt(index);
}
}

/// <summary>
/// Removes all the elements that match the conditions defined by the specified
/// predicate.
/// </summary>
/// <param name="match">
/// The <see cref="Predicate{T}"/> delegate that defines the conditions of the elements
/// to remove.
/// </param>
public void RemoveAll(Predicate<T> match)
{
List<int>? removeIndices = null;
for (int i = 0; i < _count; i++)
{
if (match(_elements[i]))
{
removeIndices ??= new List<int>();
removeIndices.Add(i);
}
}

if (removeIndices != null)
{
RemoveAtRange(removeIndices);
}
}

/// <summary>
/// Removes the <see cref="IList{T}"/> item at the specified index.
/// </summary>
Expand All @@ -438,6 +537,91 @@ public void RemoveAt(int index)
this.Count--;
}

/// <summary>
/// Removes the specified values from this list.
/// </summary>
/// <param name="index">The 0-based index into the array for the element to omit from the returned array.</param>
/// <param name="length">The number of elements to remove.</param>
public void RemoveRange(int index, int length)
{
Requires.Range(index >= 0 && index + length <= _count, nameof(index));

if (length == 0)
{
return;
}

if (index + length < this._count)
{
Array.Copy(_elements, index + length, _elements, index, this.Count - index - length);
}

this._count -= length;
}

/// <summary>
/// Removes the specified values from this list.
/// </summary>
/// <param name="items">The items to remove if matches are found in this list.</param>
public void RemoveRange(IEnumerable<T> items)
{
this.RemoveRange(items, EqualityComparer<T>.Default);
}

/// <summary>
/// Removes the specified values from this list.
/// </summary>
/// <param name="items">The items to remove if matches are found in this list.</param>
/// <param name="equalityComparer">
/// The equality comparer to use in the search.
/// If <c>null</c>, <see cref="EqualityComparer{T}.Default"/> is used.
/// </param>
public void RemoveRange(IEnumerable<T> items, IEqualityComparer<T>? equalityComparer)
{
Requires.NotNull(items, nameof(items));

var indicesToRemove = new SortedSet<int>();
foreach (var item in items)
{
int index = this.IndexOf(item, 0, _count, equalityComparer);
while (index >= 0 && !indicesToRemove.Add(index) && index + 1 < _count)
{
index = this.IndexOf(item, index + 1, equalityComparer);
}
}

this.RemoveAtRange(indicesToRemove);
}

/// <summary>
/// Replaces the first equal element in the list with the specified element.
/// </summary>
/// <param name="oldValue">The element to replace.</param>
/// <param name="newValue">The element to replace the old element with.</param>
public void Replace(T oldValue, T newValue)
{
this.Replace(oldValue, newValue, EqualityComparer<T>.Default);
}

/// <summary>
/// Replaces the first equal element in the list with the specified element.
/// </summary>
/// <param name="oldValue">The element to replace.</param>
/// <param name="newValue">The element to replace the old element with.</param>
/// <param name="equalityComparer">
/// The equality comparer to use in the search.
/// If <c>null</c>, <see cref="EqualityComparer{T}.Default"/> is used.
/// </param>
public void Replace(T oldValue, T newValue, IEqualityComparer<T>? equalityComparer)
{
int index = this.IndexOf(oldValue, 0, _count, equalityComparer);

if (index >= 0)
{
_elements[index] = newValue;
}
}

/// <summary>
/// Determines whether the <see cref="ICollection{T}"/> contains a specific value.
/// </summary>
Expand Down Expand Up @@ -477,6 +661,32 @@ public void CopyTo(T[] array, int index)
Array.Copy(_elements, 0, array, index, this.Count);
}

/// <summary>
/// Copies the contents of this array to the specified array.
/// </summary>
/// <param name="destination">The array to copy to.</param>
public void CopyTo(T[] destination)
{
Requires.NotNull(destination, nameof(destination));
Array.Copy(_elements, 0, destination, 0, this.Count);
}

/// <summary>
/// Copies the contents of this array to the specified array.
/// </summary>
/// <param name="sourceIndex">The index into this collection of the first element to copy.</param>
/// <param name="destination">The array to copy to.</param>
/// <param name="destinationIndex">The index into the destination array to which the first copied element is written.</param>
/// <param name="length">The number of elements to copy.</param>
public void CopyTo(int sourceIndex, T[] destination, int destinationIndex, int length)
{
Requires.NotNull(destination, nameof(destination));
Requires.Range(length >= 0, nameof(length));
Requires.Range(sourceIndex >= 0 && sourceIndex + length <= this.Count, nameof(sourceIndex));
Requires.Range(destinationIndex >= 0 && destinationIndex + length <= destination.Length, nameof(destinationIndex));
Array.Copy(_elements, sourceIndex, destination, destinationIndex, length);
}

/// <summary>
/// Resizes the array to accommodate the specified capacity requirement.
/// </summary>
Expand Down Expand Up @@ -565,6 +775,11 @@ public int IndexOf(T item, int startIndex, int count, IEqualityComparer<T>? equa
}
}

public int IndexOf(T item, int startIndex, IEqualityComparer<T>? equalityComparer)
{
return this.IndexOf(item, startIndex, this.Count - startIndex, equalityComparer);
}

/// <summary>
/// Searches the array for the specified item in reverse.
/// </summary>
Expand Down Expand Up @@ -792,6 +1007,32 @@ private void AddRange<TDerived>(TDerived[] items, int length) where TDerived : T
nodes[offset + i] = items[i];
}
}

private void RemoveAtRange(ICollection<int> indicesToRemove)
{
Requires.NotNull(indicesToRemove, nameof(indicesToRemove));

if (indicesToRemove.Count == 0)
{
return;
}

int copied = 0;
int removed = 0;
int lastIndexRemoved = -1;
foreach (var indexToRemove in indicesToRemove)
{
int copyLength = lastIndexRemoved == -1 ? indexToRemove : (indexToRemove - lastIndexRemoved - 1);
Array.Copy(_elements, copied + removed, _elements, copied, copyLength);
removed++;
copied += copyLength;
lastIndexRemoved = indexToRemove;
}

Array.Copy(_elements, copied + removed, _elements, copied, _elements.Length - (copied + removed));

_count -= indicesToRemove.Count;
}
}
}
}
Loading