Skip to content

Simplify collection enumerators #117563

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Simplify collection enumerators #117563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ...s/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1650,7 +1650,7 @@ bool ICollection<KeyValuePair<TKey, TValue>>.Remove(KeyValuePair<TKey, TValue> k
/// of the dictionary. The contents exposed through the enumerator may contain modifications
/// made to the dictionary after <see cref="GetEnumerator"/> was called.
/// </remarks>
IEnumerator IEnumerable.GetEnumerator() => ((ConcurrentDictionary<TKey, TValue>)this).GetEnumerator();
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

#endregion

Expand Down Expand Up @@ -2294,9 +2294,9 @@ internal Tables(VolatileNode[] buckets, object[] locks, int[] countPerLock, IEqu
/// </summary>
private sealed class DictionaryEnumerator : IDictionaryEnumerator
{
private readonly IEnumerator<KeyValuePair<TKey, TValue>> _enumerator; // Enumerator over the dictionary.
private readonly Enumerator _enumerator; // Enumerator over the dictionary.

internal DictionaryEnumerator(ConcurrentDictionary<TKey, TValue> dictionary) => _enumerator = dictionary.GetEnumerator();
internal DictionaryEnumerator(ConcurrentDictionary<TKey, TValue> dictionary) => _enumerator = new Enumerator(dictionary);

public DictionaryEntry Entry => new DictionaryEntry(_enumerator.Current.Key, _enumerator.Current.Value);

Expand Down
12 changes: 6 additions & 6 deletions src/libraries/System.Collections/src/System/Collections/Generic/LinkedList.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,16 +505,16 @@ public struct Enumerator : IEnumerator<T>, IEnumerator, ISerializable, IDeserial
private readonly LinkedList<T> _list;
private LinkedListNode<T>? _node;
private readonly int _version;
private bool _valid;
Copy link
Member

Choose a reason for hiding this comment

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

"valid" is somewhat ambiguous. How about something like _hasCurrent?

private T? _current;
private int _index;

internal Enumerator(LinkedList<T> list)
{
_list = list;
_version = list.version;
_node = list.head;
_current = default;
_index = 0;
_valid = false;
}

public T Current => _current!;
Expand All @@ -523,7 +523,7 @@ internal Enumerator(LinkedList<T> list)
{
get
{
if (_index == 0 || (_index == _list.Count + 1))
if (!_valid)
{
throw new InvalidOperationException(SR.InvalidOperation_EnumOpCantHappen);
}
Expand All @@ -541,11 +541,11 @@ public bool MoveNext()

if (_node == null)
{
_index = _list.Count + 1;
_valid = false;
return false;
}

++_index;
_valid = true;
_current = _node.item;
_node = _node.next;
if (_node == _list.head)
Expand All @@ -564,7 +564,7 @@ void IEnumerator.Reset()

_current = default;
_node = _list.head;
_index = 0;
_valid = false;
}

public void Dispose()
Expand Down
1 change: 0 additions & 1 deletion src/libraries/System.Collections/src/System/Collections/Generic/PriorityQueue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,6 @@ private bool MoveNextRare()
throw new InvalidOperationException(SR.InvalidOperation_EnumFailedVersion);
}

_index = _queue._size + 1;
_current = default;
return false;
}
Expand Down
18 changes: 9 additions & 9 deletions src/libraries/System.Collections/src/System/Collections/Generic/SortedList.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ object IDictionaryEnumerator.Key
{
get
{
if (_index == 0 || (_index == _sortedList.Count + 1))
if (_index <= 0)
{
throw new InvalidOperationException(SR.InvalidOperation_EnumOpCantHappen);
}
Expand All @@ -801,7 +801,7 @@ public bool MoveNext()
return true;
}

_index = _sortedList.Count + 1;
_index = -1;
_key = default;
_value = default;
return false;
Expand All @@ -811,7 +811,7 @@ DictionaryEntry IDictionaryEnumerator.Entry
{
get
{
if (_index == 0 || (_index == _sortedList.Count + 1))
if (_index <= 0)
{
throw new InvalidOperationException(SR.InvalidOperation_EnumOpCantHappen);
}
Expand All @@ -826,7 +826,7 @@ DictionaryEntry IDictionaryEnumerator.Entry
{
get
{
if (_index == 0 || (_index == _sortedList.Count + 1))
if (_index <= 0)
{
throw new InvalidOperationException(SR.InvalidOperation_EnumOpCantHappen);
}
Expand All @@ -846,7 +846,7 @@ DictionaryEntry IDictionaryEnumerator.Entry
{
get
{
if (_index == 0 || (_index == _sortedList.Count + 1))
if (_index <= 0)
{
throw new InvalidOperationException(SR.InvalidOperation_EnumOpCantHappen);
}
Expand Down Expand Up @@ -901,7 +901,7 @@ public bool MoveNext()
return true;
}

_index = _sortedList.Count + 1;
_index = -1;
_currentKey = default;
return false;
}
Expand All @@ -912,7 +912,7 @@ public bool MoveNext()
{
get
{
if (_index == 0 || (_index == _sortedList.Count + 1))
if (_index <= 0)
{
throw new InvalidOperationException(SR.InvalidOperation_EnumOpCantHappen);
}
Expand Down Expand Up @@ -965,7 +965,7 @@ public bool MoveNext()
return true;
}

_index = _sortedList.Count + 1;
_index = -1;
_currentValue = default;
return false;
}
Expand All @@ -976,7 +976,7 @@ public bool MoveNext()
{
get
{
if (_index == 0 || (_index == _sortedList.Count + 1))
if (_index <= 0)
{
throw new InvalidOperationException(SR.InvalidOperation_EnumOpCantHappen);
}
Expand Down
3 changes: 1 addition & 2 deletions src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1832,11 +1832,10 @@ private bool HasChildren(Node child1, Node child2)
public struct Enumerator : IEnumerator<T>, IEnumerator, ISerializable, IDeserializationCallback
{
private readonly SortedSet<T> _tree;
private readonly int _version;

private readonly Stack<Node> _stack;
private Node? _current;

private readonly int _version;
private readonly bool _reverse;

internal Enumerator(SortedSet<T> set)
Expand Down
18 changes: 5 additions & 13 deletions src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@ public object Clone()
public bool MoveNext()
{
nint index = _index + 1;
nuint length = _array.NativeLength;
if ((nuint)index >= length)
if ((nuint)index >= _array.NativeLength)
{
_index = (nint)length;
_index = -2;
return false;
}
_index = index;
Expand All @@ -47,14 +46,7 @@ public object? Current

if ((nuint)index >= array.NativeLength)
{
if (index < 0)
{
ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumNotStarted();
}
else
{
ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumEnded();
}
ThrowHelper.ThrowInvalidOperationException_EnumCurrent((int)index);
}

return array.InternalGetValue(index);
Expand Down Expand Up @@ -140,7 +132,7 @@ public object Current
{
get
{
ThrowHelper.ThrowInvalidOperationException_EnumCurrent(-1);
ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumNotStarted();
return default;
}
}
Expand All @@ -167,7 +159,7 @@ private GenericEmptyEnumerator() { }
{
get
{
ThrowHelper.ThrowInvalidOperationException_EnumCurrent(-1);
ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumNotStarted();
return default;
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/libraries/System.Private.CoreLib/src/System/ArraySegment.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,11 +300,13 @@ internal Enumerator(ArraySegment<T> arraySegment)

public bool MoveNext()
{
if (_current < _end)
int current = _current + 1;
if ((uint)current < (uint)_end)
{
_current++;
return _current < _end;
_current = current;
return true;
}
_current = -2;
return false;
}

Expand All @@ -313,9 +315,7 @@ public T Current
get
{
if (_current < _start)
ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumNotStarted();
if (_current >= _end)
ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumEnded();
ThrowHelper.ThrowInvalidOperationException_EnumCurrent(_current - _start);
return _array![_current];
}
}
Expand Down
8 changes: 3 additions & 5 deletions src/libraries/System.Private.CoreLib/src/System/CharEnumerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,13 @@ public sealed class CharEnumerator : IEnumerator, IEnumerator<char>, IDisposable
public bool MoveNext()
{
int index = _index + 1;
int length = _str.Length;

if (index < length)
if ((uint)index < (uint)_str.Length)
{
_index = index;
return true;
}

_index = length;
_index = -2;
return false;
}

Expand All @@ -43,7 +41,7 @@ public char Current
string s = _str;
if ((uint)index >= (uint)s.Length)
{
ThrowHelper.ThrowInvalidOperationException_EnumCurrent(_index);
ThrowHelper.ThrowInvalidOperationException_EnumCurrent(index);
}

return s[index];
Expand Down
52 changes: 16 additions & 36 deletions src/libraries/System.Private.CoreLib/src/System/Collections/ArrayList.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2553,16 +2553,12 @@ private sealed class ArrayListEnumeratorSimple : IEnumerator, ICloneable
private readonly int _version;
private object? _currentElement;
private readonly bool _isArrayList;
// this object is used to indicate enumeration has not started or has terminated
private static readonly object s_dummyObject = new object();

internal ArrayListEnumeratorSimple(ArrayList list)
{
_list = list;
_index = -1;
_version = list._version;
_isArrayList = (list.GetType() == typeof(ArrayList));
_currentElement = s_dummyObject;
}

public object Clone() => MemberwiseClone();
Expand All @@ -2571,69 +2567,53 @@ public bool MoveNext()
{
if (_version != _list._version)
{
throw new InvalidOperationException(SR.InvalidOperation_EnumFailedVersion);
ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumFailedVersion();
}

if (_isArrayList)
{ // avoid calling virtual methods if we are operating on ArrayList to improve performance
if (_index < _list._size - 1)
if ((uint)_index < (uint)_list._size)
{
_currentElement = _list._items[++_index];
_currentElement = _list._items[_index++];
return true;
}
else
{
_currentElement = s_dummyObject;
_index = _list._size;
return false;
}
}
else
{
if (_index < _list.Count - 1)
if ((uint)_index < (uint)_list.Count)
{
_currentElement = _list[++_index];
_currentElement = _list[_index++];
return true;
}
else
{
_index = _list.Count;
_currentElement = s_dummyObject;
return false;
}
}

_index = -1;
_currentElement = null;
return false;
}

public object? Current
{
get
{
object? temp = _currentElement;
if (s_dummyObject == temp)
{ // check if enumeration has not started or has terminated
if (_index == -1)
{
throw new InvalidOperationException(SR.InvalidOperation_EnumNotStarted);
}
else
{
throw new InvalidOperationException(SR.InvalidOperation_EnumEnded);
}
if (_index <= 0)
{
ThrowHelper.ThrowInvalidOperationException_EnumCurrent(_index - 1);
}

return temp;
return _currentElement;
}
}

public void Reset()
{
if (_version != _list._version)
{
throw new InvalidOperationException(SR.InvalidOperation_EnumFailedVersion);
ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumFailedVersion();
}

_currentElement = s_dummyObject;
_index = -1;
_currentElement = null;
_index = 0;
}
}

Expand Down
Loading
Loading