From daf8034fbb79ecda0565eb7f1e96613db0f87039 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 23:34:21 +0000 Subject: [PATCH 1/3] fix: throw when an array-backed container is modified during enumeration [patch] ContiguousCollection, ContiguousSet, ContiguousMap, OrderedMap and InsertionOrderMap enumerated with a bare index loop and no version counter. Removing or adding during a foreach silently skipped or repeated elements instead of throwing InvalidOperationException, unlike their List-backed siblings. Each now keeps a version, bumped by every add, insert, remove, clear and indexer set. Their enumerators, including the maps' Keys and Values, capture it when GetEnumerator() is called and throw on the next MoveNext once it has changed. A set Add of a duplicate, or a Remove of a missing item, changes nothing and does not invalidate enumeration. RingBuffer is left as it is: it is written to in real time, and the issue marks it optional. Fixes ktsu-dev/Containers#70 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PtWVWaDh3nqJL1eKD7vFxU --- Containers.Test/ContiguousCollectionTests.cs | 86 ++++++++++++++++++++ Containers.Test/ContiguousMapTests.cs | 54 ++++++++++++ Containers.Test/ContiguousSetTests.cs | 63 ++++++++++++++ Containers.Test/InsertionOrderMapTests.cs | 54 ++++++++++++ Containers.Test/OrderedMapTests.cs | 54 ++++++++++++ Containers/ContiguousCollection.cs | 23 +++++- Containers/ContiguousMap.cs | 51 ++++++++++-- Containers/ContiguousSet.cs | 21 ++++- Containers/Enumeration.cs | 24 ++++++ Containers/InsertionOrderMap.cs | 51 ++++++++++-- Containers/OrderedMap.cs | 57 +++++++++++-- 11 files changed, 512 insertions(+), 26 deletions(-) create mode 100644 Containers/Enumeration.cs diff --git a/Containers.Test/ContiguousCollectionTests.cs b/Containers.Test/ContiguousCollectionTests.cs index ce3930b..aeac790 100644 --- a/Containers.Test/ContiguousCollectionTests.cs +++ b/Containers.Test/ContiguousCollectionTests.cs @@ -528,4 +528,90 @@ public void WorksWithReferenceTypes() Assert.AreEqual("alpha", collection[1]); Assert.AreEqual("bravo", collection[2]); } + + [TestMethod] + public void Enumerate_RemovingDuringForeach_Throws() + { + ContiguousCollection collection = [1, 2, 3, 4]; + + Assert.ThrowsExactly(() => + { + foreach (int item in collection) + { + collection.Remove(item); + } + }); + } + + [TestMethod] + public void Enumerate_AddingDuringForeach_Throws() + { + ContiguousCollection collection = [1, 2]; + + Assert.ThrowsExactly(() => + { + foreach (int item in collection) + { + collection.Add(item); + } + }); + } + + [TestMethod] + public void Enumerate_ChangesMadeThroughEveryMutator_Throw() + { + Action>[] mutations = + [ + c => c.Add(9), + c => c.Insert(0, 9), + c => c.Remove(1), + c => c.RemoveAt(0), + c => c.Clear(), + c => c[0] = 9, + ]; + + foreach (Action> mutate in mutations) + { + ContiguousCollection collection = [1, 2, 3]; + using IEnumerator enumerator = collection.GetEnumerator(); + Assert.IsTrue(enumerator.MoveNext()); + + mutate(collection); + + Assert.ThrowsExactly(() => enumerator.MoveNext()); + } + } + + [TestMethod] + public void Enumerate_ChangedBeforeTheFirstMoveNext_Throws() + { + ContiguousCollection collection = [1, 2, 3]; + using IEnumerator enumerator = collection.GetEnumerator(); + + collection.Add(4); + + Assert.ThrowsExactly(() => enumerator.MoveNext()); + } + + [TestMethod] + public void Enumerate_ChangedAfterTheLastElement_Throws() + { + ContiguousCollection collection = [1, 2]; + using IEnumerator enumerator = collection.GetEnumerator(); + Assert.IsTrue(enumerator.MoveNext()); + Assert.IsTrue(enumerator.MoveNext()); + + collection.Add(3); + + Assert.ThrowsExactly(() => enumerator.MoveNext()); + } + + [TestMethod] + public void Enumerate_Unchanged_VisitsEveryElement() + { + ContiguousCollection collection = [1, 2, 3]; + collection.Add(4); + + Assert.AreSequenceEqual([1, 2, 3, 4], collection); + } } diff --git a/Containers.Test/ContiguousMapTests.cs b/Containers.Test/ContiguousMapTests.cs index 42f20f2..54ac748 100644 --- a/Containers.Test/ContiguousMapTests.cs +++ b/Containers.Test/ContiguousMapTests.cs @@ -593,4 +593,58 @@ public void Entry_KeyAndValue_ExposeConstructorArguments() Assert.AreEqual(7, entry.Key); Assert.AreEqual("seven", entry.Value); } + + [TestMethod] + public void Enumerate_RemovingDuringForeach_Throws() + { + ContiguousMap map = new() { [1] = "one", [2] = "two", [3] = "three", [4] = "four" }; + + Assert.ThrowsExactly(() => + { + foreach (KeyValuePair pair in map) + { + map.Remove(pair.Key); + } + }); + } + + [TestMethod] + public void Enumerate_ChangesMadeThroughEveryMutator_Throw() + { + Action>[] mutations = + [ + m => m.Add(9, "nine"), + m => m[9] = "nine", + m => m[1] = "uno", + m => m.Remove(1), + m => m.Remove(new KeyValuePair(1, "one")), + m => m.Clear(), + ]; + + foreach (Action> mutate in mutations) + { + ContiguousMap map = new() { [1] = "one", [2] = "two" }; + using IEnumerator> pairs = map.GetEnumerator(); + using IEnumerator keys = map.Keys.GetEnumerator(); + using IEnumerator values = map.Values.GetEnumerator(); + Assert.IsTrue(pairs.MoveNext()); + Assert.IsTrue(keys.MoveNext()); + Assert.IsTrue(values.MoveNext()); + + mutate(map); + + Assert.ThrowsExactly(() => pairs.MoveNext()); + Assert.ThrowsExactly(() => keys.MoveNext()); + Assert.ThrowsExactly(() => values.MoveNext()); + } + } + + [TestMethod] + public void Enumerate_Unchanged_VisitsEveryEntry() + { + ContiguousMap map = new() { [1] = "one", [2] = "two" }; + + Assert.AreSequenceEqual([1, 2], map.Keys); + Assert.AreSequenceEqual(["one", "two"], map.Values); + } } diff --git a/Containers.Test/ContiguousSetTests.cs b/Containers.Test/ContiguousSetTests.cs index 3e1c8c3..fd1d91a 100644 --- a/Containers.Test/ContiguousSetTests.cs +++ b/Containers.Test/ContiguousSetTests.cs @@ -495,4 +495,67 @@ public void Remove_WithCustomComparer_ThenAdd_KeepsSingleElement() string[] expectedItems = ["apple"]; Assert.AreSequenceEqual(expectedItems, set); } + + [TestMethod] + public void Enumerate_RemovingDuringForeach_Throws() + { + ContiguousSet set = [1, 2, 3, 4]; + + Assert.ThrowsExactly(() => + { + foreach (int item in set) + { + set.Remove(item); + } + }); + } + + [TestMethod] + public void Enumerate_ChangesMadeThroughEveryMutator_Throw() + { + Action>[] mutations = + [ + s => s.Add(9), + s => s.Remove(1), + s => s.Clear(), + ]; + + foreach (Action> mutate in mutations) + { + ContiguousSet set = [1, 2, 3]; + using IEnumerator enumerator = set.GetEnumerator(); + Assert.IsTrue(enumerator.MoveNext()); + + mutate(set); + + Assert.ThrowsExactly(() => enumerator.MoveNext()); + } + } + + [TestMethod] + public void Enumerate_AddingADuplicateOrRemovingAMissingItem_DoesNotThrow() + { + // Neither changes the set, so an enumeration in progress is still valid. + ContiguousSet set = [1, 2, 3]; + List seen = []; + + foreach (int item in set) + { + set.Add(item); + set.Remove(99); + seen.Add(item); + } + + Assert.AreSequenceEqual([1, 2, 3], seen); + } + + [TestMethod] + public void UnionWith_Itself_DoesNotThrow() + { + ContiguousSet set = [1, 2, 3]; + + set.UnionWith(set); + + Assert.AreSequenceEqual([1, 2, 3], set); + } } diff --git a/Containers.Test/InsertionOrderMapTests.cs b/Containers.Test/InsertionOrderMapTests.cs index 9c92da1..2302bb6 100644 --- a/Containers.Test/InsertionOrderMapTests.cs +++ b/Containers.Test/InsertionOrderMapTests.cs @@ -424,4 +424,58 @@ public void WorksWithCustomTypes() string[] expectedKeyOrder = ["charlie", "alpha", "bravo"]; Assert.AreSequenceEqual(expectedKeyOrder, map.Keys); } + + [TestMethod] + public void Enumerate_RemovingDuringForeach_Throws() + { + InsertionOrderMap map = new() { [1] = "one", [2] = "two", [3] = "three", [4] = "four" }; + + Assert.ThrowsExactly(() => + { + foreach (KeyValuePair pair in map) + { + map.Remove(pair.Key); + } + }); + } + + [TestMethod] + public void Enumerate_ChangesMadeThroughEveryMutator_Throw() + { + Action>[] mutations = + [ + m => m.Add(9, "nine"), + m => m[9] = "nine", + m => m[1] = "uno", + m => m.Remove(1), + m => m.Remove(new KeyValuePair(1, "one")), + m => m.Clear(), + ]; + + foreach (Action> mutate in mutations) + { + InsertionOrderMap map = new() { [1] = "one", [2] = "two" }; + using IEnumerator> pairs = map.GetEnumerator(); + using IEnumerator keys = map.Keys.GetEnumerator(); + using IEnumerator values = map.Values.GetEnumerator(); + Assert.IsTrue(pairs.MoveNext()); + Assert.IsTrue(keys.MoveNext()); + Assert.IsTrue(values.MoveNext()); + + mutate(map); + + Assert.ThrowsExactly(() => pairs.MoveNext()); + Assert.ThrowsExactly(() => keys.MoveNext()); + Assert.ThrowsExactly(() => values.MoveNext()); + } + } + + [TestMethod] + public void Enumerate_Unchanged_VisitsEveryEntry() + { + InsertionOrderMap map = new() { [1] = "one", [2] = "two" }; + + Assert.AreSequenceEqual([1, 2], map.Keys); + Assert.AreSequenceEqual(["one", "two"], map.Values); + } } diff --git a/Containers.Test/OrderedMapTests.cs b/Containers.Test/OrderedMapTests.cs index 900f601..59510d0 100644 --- a/Containers.Test/OrderedMapTests.cs +++ b/Containers.Test/OrderedMapTests.cs @@ -517,4 +517,58 @@ public void LargeCollection_MaintainsOrder() Assert.AreEqual(i + 1, keys[i]); } } + + [TestMethod] + public void Enumerate_RemovingDuringForeach_Throws() + { + OrderedMap map = new() { [1] = "one", [2] = "two", [3] = "three", [4] = "four" }; + + Assert.ThrowsExactly(() => + { + foreach (KeyValuePair pair in map) + { + map.Remove(pair.Key); + } + }); + } + + [TestMethod] + public void Enumerate_ChangesMadeThroughEveryMutator_Throw() + { + Action>[] mutations = + [ + m => m.Add(9, "nine"), + m => m[9] = "nine", + m => m[1] = "uno", + m => m.Remove(1), + m => m.Remove(new KeyValuePair(1, "one")), + m => m.Clear(), + ]; + + foreach (Action> mutate in mutations) + { + OrderedMap map = new() { [1] = "one", [2] = "two" }; + using IEnumerator> pairs = map.GetEnumerator(); + using IEnumerator keys = map.Keys.GetEnumerator(); + using IEnumerator values = map.Values.GetEnumerator(); + Assert.IsTrue(pairs.MoveNext()); + Assert.IsTrue(keys.MoveNext()); + Assert.IsTrue(values.MoveNext()); + + mutate(map); + + Assert.ThrowsExactly(() => pairs.MoveNext()); + Assert.ThrowsExactly(() => keys.MoveNext()); + Assert.ThrowsExactly(() => values.MoveNext()); + } + } + + [TestMethod] + public void Enumerate_Unchanged_VisitsEveryEntry() + { + OrderedMap map = new() { [1] = "one", [2] = "two" }; + + Assert.AreSequenceEqual([1, 2], map.Keys); + Assert.AreSequenceEqual(["one", "two"], map.Values); + } } diff --git a/Containers/ContiguousCollection.cs b/Containers/ContiguousCollection.cs index 3211827..4764db5 100644 --- a/Containers/ContiguousCollection.cs +++ b/Containers/ContiguousCollection.cs @@ -49,6 +49,11 @@ public class ContiguousCollection : ICollection, IReadOnlyList /// private T[] items; + /// + /// Incremented by every change, so an enumerator can tell the collection changed under it. + /// + private int version; + /// /// The default initial capacity for the collection. /// @@ -88,6 +93,7 @@ public T this[int index] ArgumentOutOfRangeException.ThrowIfNegative(index); ArgumentOutOfRangeException.ThrowIfGreaterThanOrEqual(index, Count); items[index] = value; + version++; } } @@ -158,6 +164,7 @@ public void Add(T item) items[Count] = item; Count++; + version++; } /// @@ -175,6 +182,7 @@ public void Clear() Array.Clear(items, 0, Count); } Count = 0; + version++; } /// @@ -241,6 +249,7 @@ public void RemoveAt(int index) ArgumentOutOfRangeException.ThrowIfGreaterThanOrEqual(index, Count); Count--; + version++; if (index < Count) { Array.Copy(items, index + 1, items, index, Count - index); @@ -294,6 +303,7 @@ public void Insert(int index, T item) items[index] = item; Count++; + version++; } /// @@ -341,9 +351,18 @@ public void TrimExcess() /// public IEnumerator GetEnumerator() { - for (int i = 0; i < Count; i++) + int expected = version; + return Enumerate(); + + IEnumerator Enumerate() { - yield return items[i]; + for (int i = 0; i < Count; i++) + { + Enumeration.ThrowIfModified(expected, version); + yield return items[i]; + } + + Enumeration.ThrowIfModified(expected, version); } } diff --git a/Containers/ContiguousMap.cs b/Containers/ContiguousMap.cs index 299d367..dec8f68 100644 --- a/Containers/ContiguousMap.cs +++ b/Containers/ContiguousMap.cs @@ -126,6 +126,11 @@ public override int GetHashCode() /// private Entry[] items; + /// + /// Incremented by every change, so an enumerator can tell the collection changed under it. + /// + private int version; + /// /// The internal dictionary used for fast key-based lookups to array indices. /// @@ -176,6 +181,7 @@ public TValue this[TKey key] { // Key exists, update the value and keep the key that was stored first items[index] = new Entry(items[index].Key, value); + version++; } else { @@ -189,6 +195,7 @@ public TValue this[TKey key] items[index] = new Entry(key, value); keyToIndex[key] = index; Count++; + version++; } } } @@ -342,6 +349,7 @@ public void Add(TKey key, TValue value) items[index] = new Entry(key, value); keyToIndex[key] = index; Count++; + version++; } /// @@ -365,6 +373,7 @@ public bool Remove(TKey key) // Remove from the array by shifting elements Count--; + version++; if (index < Count) { Array.Copy(items, index + 1, items, index, Count - index); @@ -452,6 +461,7 @@ public void Clear() Array.Clear(items, 0, Count); } Count = 0; + version++; keyToIndex.Clear(); } @@ -505,10 +515,19 @@ public bool Remove(KeyValuePair item) => /// public IEnumerator> GetEnumerator() { - for (int i = 0; i < Count; i++) + int expected = version; + return Enumerate(); + + IEnumerator> Enumerate() { - Entry entry = items[i]; - yield return new KeyValuePair(entry.Key, entry.Value); + for (int i = 0; i < Count; i++) + { + Entry entry = items[i]; + Enumeration.ThrowIfModified(expected, version); + yield return new KeyValuePair(entry.Key, entry.Value); + } + + Enumeration.ThrowIfModified(expected, version); } } @@ -682,9 +701,18 @@ public void CopyTo(TKey[] array, int arrayIndex) public IEnumerator GetEnumerator() { - for (int i = 0; i < map.Count; i++) + int expected = map.version; + return Enumerate(); + + IEnumerator Enumerate() { - yield return map.items[i].Key; + for (int i = 0; i < map.Count; i++) + { + Enumeration.ThrowIfModified(expected, map.version); + yield return map.items[i].Key; + } + + Enumeration.ThrowIfModified(expected, map.version); } } @@ -736,9 +764,18 @@ public void CopyTo(TValue[] array, int arrayIndex) public IEnumerator GetEnumerator() { - for (int i = 0; i < map.Count; i++) + int expected = map.version; + return Enumerate(); + + IEnumerator Enumerate() { - yield return map.items[i].Value; + for (int i = 0; i < map.Count; i++) + { + Enumeration.ThrowIfModified(expected, map.version); + yield return map.items[i].Value; + } + + Enumeration.ThrowIfModified(expected, map.version); } } diff --git a/Containers/ContiguousSet.cs b/Containers/ContiguousSet.cs index f76fe87..9e7a766 100644 --- a/Containers/ContiguousSet.cs +++ b/Containers/ContiguousSet.cs @@ -56,6 +56,11 @@ public class ContiguousSet : ISet /// private T[] items; + /// + /// Incremented by every change, so an enumerator can tell the collection changed under it. + /// + private int version; + /// /// The internal hash set used for fast uniqueness checks. /// @@ -208,6 +213,7 @@ public bool Add(T item) items[Count] = item; Count++; + version++; return true; } @@ -232,6 +238,7 @@ public void Clear() Array.Clear(items, 0, Count); } Count = 0; + version++; uniquenessSet.Clear(); } @@ -297,6 +304,7 @@ public bool Remove(T item) if (index >= 0) { Count--; + version++; if (index < Count) { Array.Copy(items, index + 1, items, index, Count - index); @@ -324,9 +332,18 @@ public bool Remove(T item) /// public IEnumerator GetEnumerator() { - for (int i = 0; i < Count; i++) + int expected = version; + return Enumerate(); + + IEnumerator Enumerate() { - yield return items[i]; + for (int i = 0; i < Count; i++) + { + Enumeration.ThrowIfModified(expected, version); + yield return items[i]; + } + + Enumeration.ThrowIfModified(expected, version); } } diff --git a/Containers/Enumeration.cs b/Containers/Enumeration.cs new file mode 100644 index 0000000..cbae8e4 --- /dev/null +++ b/Containers/Enumeration.cs @@ -0,0 +1,24 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Containers; + +/// +/// Shared checks for the enumerators of the containers that keep a version counter. +/// +internal static class Enumeration +{ + /// + /// Throws when a collection has changed since its enumerator was created, as the + /// contract expects. + /// + /// The version the collection had when enumeration began. + /// The version the collection has now. + /// The collection was modified. + internal static void ThrowIfModified(int expected, int actual) + { + if (expected != actual) + { + throw new InvalidOperationException("Collection was modified; enumeration operation may not execute."); + } + } +} diff --git a/Containers/InsertionOrderMap.cs b/Containers/InsertionOrderMap.cs index 30e621a..e09efae 100644 --- a/Containers/InsertionOrderMap.cs +++ b/Containers/InsertionOrderMap.cs @@ -50,6 +50,11 @@ private struct Entry(TKey key, TValue value) /// private readonly List items; + /// + /// Incremented by every change, so an enumerator can tell the collection changed under it. + /// + private int version; + /// /// The internal dictionary used for fast key-based lookups. /// @@ -92,12 +97,14 @@ public TValue this[TKey key] Entry entry = items[index]; entry.Value = value; items[index] = entry; + version++; } else { // Key doesn't exist, add new entry index = items.Count; items.Add(new Entry(key, value)); + version++; keyToIndex[key] = index; } } @@ -237,6 +244,7 @@ public void Add(TKey key, TValue value) int index = items.Count; items.Add(new Entry(key, value)); + version++; keyToIndex[key] = index; } @@ -260,6 +268,7 @@ public bool Remove(TKey key) // Remove from the list items.RemoveAt(index); + version++; keyToIndex.Remove(key); // Update indices in the dictionary for all elements after the removed one @@ -332,6 +341,7 @@ public bool TryGetValue(TKey key, public void Clear() { items.Clear(); + version++; keyToIndex.Clear(); } @@ -382,10 +392,19 @@ public bool Remove(KeyValuePair item) => /// An enumerator for the map. public IEnumerator> GetEnumerator() { - for (int i = 0; i < items.Count; i++) + int expected = version; + return Enumerate(); + + IEnumerator> Enumerate() { - Entry entry = items[i]; - yield return new KeyValuePair(entry.Key, entry.Value); + for (int i = 0; i < items.Count; i++) + { + Entry entry = items[i]; + Enumeration.ThrowIfModified(expected, version); + yield return new KeyValuePair(entry.Key, entry.Value); + } + + Enumeration.ThrowIfModified(expected, version); } } @@ -440,9 +459,18 @@ public void CopyTo(TKey[] array, int arrayIndex) public IEnumerator GetEnumerator() { - for (int i = 0; i < map.items.Count; i++) + int expected = map.version; + return Enumerate(); + + IEnumerator Enumerate() { - yield return map.items[i].Key; + for (int i = 0; i < map.items.Count; i++) + { + Enumeration.ThrowIfModified(expected, map.version); + yield return map.items[i].Key; + } + + Enumeration.ThrowIfModified(expected, map.version); } } @@ -485,9 +513,18 @@ public void CopyTo(TValue[] array, int arrayIndex) public IEnumerator GetEnumerator() { - for (int i = 0; i < map.items.Count; i++) + int expected = map.version; + return Enumerate(); + + IEnumerator Enumerate() { - yield return map.items[i].Value; + for (int i = 0; i < map.items.Count; i++) + { + Enumeration.ThrowIfModified(expected, map.version); + yield return map.items[i].Value; + } + + Enumeration.ThrowIfModified(expected, map.version); } } diff --git a/Containers/OrderedMap.cs b/Containers/OrderedMap.cs index 39759d1..c438916 100644 --- a/Containers/OrderedMap.cs +++ b/Containers/OrderedMap.cs @@ -59,6 +59,11 @@ comparer is null ) : []; + /// + /// Incremented by every change, so an enumerator can tell the collection changed under it. + /// + private int version; + /// /// The comparer used to maintain sorted order by key. /// @@ -103,12 +108,14 @@ public TValue this[TKey key] Entry entry = items[index]; entry.Value = value; items[index] = entry; + version++; } else { // Key doesn't exist, add new entry index = ~index; // Convert to insertion point items.Insert(index, new Entry(key, value)); + version++; } } } @@ -239,6 +246,7 @@ public void Add(TKey key, TValue value) index = ~index; // Convert to insertion point items.Insert(index, new Entry(key, value)); + version++; } /// @@ -258,6 +266,7 @@ public bool Remove(TKey key) } items.RemoveAt(index); + version++; return true; } @@ -315,7 +324,11 @@ public bool TryGetValue(TKey key, /// /// Removes all key-value pairs from the map. /// - public void Clear() => items.Clear(); + public void Clear() + { + items.Clear(); + version++; + } /// /// Determines whether the map contains a specific key-value pair. @@ -382,6 +395,7 @@ public bool Remove(KeyValuePair item) } items.RemoveAt(index); + version++; return true; } @@ -391,10 +405,19 @@ public bool Remove(KeyValuePair item) /// An enumerator that can be used to iterate through the map. public IEnumerator> GetEnumerator() { - for (int i = 0; i < items.Count; i++) + int expected = version; + return Enumerate(); + + IEnumerator> Enumerate() { - Entry entry = items[i]; - yield return new KeyValuePair(entry.Key, entry.Value); + for (int i = 0; i < items.Count; i++) + { + Entry entry = items[i]; + Enumeration.ThrowIfModified(expected, version); + yield return new KeyValuePair(entry.Key, entry.Value); + } + + Enumeration.ThrowIfModified(expected, version); } } @@ -491,9 +514,18 @@ public void CopyTo(TKey[] array, int arrayIndex) public IEnumerator GetEnumerator() { - for (int i = 0; i < map.items.Count; i++) + int expected = map.version; + return Enumerate(); + + IEnumerator Enumerate() { - yield return map.items[i].Key; + for (int i = 0; i < map.items.Count; i++) + { + Enumeration.ThrowIfModified(expected, map.version); + yield return map.items[i].Key; + } + + Enumeration.ThrowIfModified(expected, map.version); } } @@ -543,9 +575,18 @@ public void CopyTo(TValue[] array, int arrayIndex) public IEnumerator GetEnumerator() { - for (int i = 0; i < map.items.Count; i++) + int expected = map.version; + return Enumerate(); + + IEnumerator Enumerate() { - yield return map.items[i].Value; + for (int i = 0; i < map.items.Count; i++) + { + Enumeration.ThrowIfModified(expected, map.version); + yield return map.items[i].Value; + } + + Enumeration.ThrowIfModified(expected, map.version); } } From b817d3f0fb5b3706b6eaa561fc8ebfabfae456fa Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 23:43:47 +0000 Subject: [PATCH 2/3] Tighten the versioned enumerators and share the map tests Each enumerator is now a one-line GetEnumerator that passes the version to a private iterator, rather than a wrapper around a local function. That removes the repeated block Sonar counted as duplicated new code. The map enumerators check the version before reading the entry, and the three identical map test sets now call one shared assertion helper. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PtWVWaDh3nqJL1eKD7vFxU --- Containers.Test/ContiguousMapTests.cs | 54 ++------------- Containers.Test/InsertionOrderMapTests.cs | 54 ++------------- Containers.Test/MapEnumerationAssertions.cs | 74 +++++++++++++++++++++ Containers.Test/OrderedMapTests.cs | 54 ++------------- Containers/ContiguousCollection.cs | 18 ++--- Containers/ContiguousMap.cs | 56 ++++++---------- Containers/ContiguousSet.cs | 18 ++--- Containers/InsertionOrderMap.cs | 56 ++++++---------- Containers/OrderedMap.cs | 56 ++++++---------- 9 files changed, 172 insertions(+), 268 deletions(-) create mode 100644 Containers.Test/MapEnumerationAssertions.cs diff --git a/Containers.Test/ContiguousMapTests.cs b/Containers.Test/ContiguousMapTests.cs index 54ac748..23d504a 100644 --- a/Containers.Test/ContiguousMapTests.cs +++ b/Containers.Test/ContiguousMapTests.cs @@ -595,56 +595,14 @@ public void Entry_KeyAndValue_ExposeConstructorArguments() } [TestMethod] - public void Enumerate_RemovingDuringForeach_Throws() - { - ContiguousMap map = new() { [1] = "one", [2] = "two", [3] = "three", [4] = "four" }; - - Assert.ThrowsExactly(() => - { - foreach (KeyValuePair pair in map) - { - map.Remove(pair.Key); - } - }); - } + public void Enumerate_RemovingDuringForeach_Throws() => + MapEnumerationAssertions.RemovingDuringForeachThrows(() => new ContiguousMap()); [TestMethod] - public void Enumerate_ChangesMadeThroughEveryMutator_Throw() - { - Action>[] mutations = - [ - m => m.Add(9, "nine"), - m => m[9] = "nine", - m => m[1] = "uno", - m => m.Remove(1), - m => m.Remove(new KeyValuePair(1, "one")), - m => m.Clear(), - ]; - - foreach (Action> mutate in mutations) - { - ContiguousMap map = new() { [1] = "one", [2] = "two" }; - using IEnumerator> pairs = map.GetEnumerator(); - using IEnumerator keys = map.Keys.GetEnumerator(); - using IEnumerator values = map.Values.GetEnumerator(); - Assert.IsTrue(pairs.MoveNext()); - Assert.IsTrue(keys.MoveNext()); - Assert.IsTrue(values.MoveNext()); - - mutate(map); - - Assert.ThrowsExactly(() => pairs.MoveNext()); - Assert.ThrowsExactly(() => keys.MoveNext()); - Assert.ThrowsExactly(() => values.MoveNext()); - } - } + public void Enumerate_ChangesMadeThroughEveryMutator_Throw() => + MapEnumerationAssertions.EveryMutatorInvalidatesEnumerators(() => new ContiguousMap()); [TestMethod] - public void Enumerate_Unchanged_VisitsEveryEntry() - { - ContiguousMap map = new() { [1] = "one", [2] = "two" }; - - Assert.AreSequenceEqual([1, 2], map.Keys); - Assert.AreSequenceEqual(["one", "two"], map.Values); - } + public void Enumerate_Unchanged_VisitsEveryEntry() => + MapEnumerationAssertions.UnchangedEnumerationVisitsEveryEntry(() => new ContiguousMap()); } diff --git a/Containers.Test/InsertionOrderMapTests.cs b/Containers.Test/InsertionOrderMapTests.cs index 2302bb6..15a1f71 100644 --- a/Containers.Test/InsertionOrderMapTests.cs +++ b/Containers.Test/InsertionOrderMapTests.cs @@ -426,56 +426,14 @@ public void WorksWithCustomTypes() } [TestMethod] - public void Enumerate_RemovingDuringForeach_Throws() - { - InsertionOrderMap map = new() { [1] = "one", [2] = "two", [3] = "three", [4] = "four" }; - - Assert.ThrowsExactly(() => - { - foreach (KeyValuePair pair in map) - { - map.Remove(pair.Key); - } - }); - } + public void Enumerate_RemovingDuringForeach_Throws() => + MapEnumerationAssertions.RemovingDuringForeachThrows(() => new InsertionOrderMap()); [TestMethod] - public void Enumerate_ChangesMadeThroughEveryMutator_Throw() - { - Action>[] mutations = - [ - m => m.Add(9, "nine"), - m => m[9] = "nine", - m => m[1] = "uno", - m => m.Remove(1), - m => m.Remove(new KeyValuePair(1, "one")), - m => m.Clear(), - ]; - - foreach (Action> mutate in mutations) - { - InsertionOrderMap map = new() { [1] = "one", [2] = "two" }; - using IEnumerator> pairs = map.GetEnumerator(); - using IEnumerator keys = map.Keys.GetEnumerator(); - using IEnumerator values = map.Values.GetEnumerator(); - Assert.IsTrue(pairs.MoveNext()); - Assert.IsTrue(keys.MoveNext()); - Assert.IsTrue(values.MoveNext()); - - mutate(map); - - Assert.ThrowsExactly(() => pairs.MoveNext()); - Assert.ThrowsExactly(() => keys.MoveNext()); - Assert.ThrowsExactly(() => values.MoveNext()); - } - } + public void Enumerate_ChangesMadeThroughEveryMutator_Throw() => + MapEnumerationAssertions.EveryMutatorInvalidatesEnumerators(() => new InsertionOrderMap()); [TestMethod] - public void Enumerate_Unchanged_VisitsEveryEntry() - { - InsertionOrderMap map = new() { [1] = "one", [2] = "two" }; - - Assert.AreSequenceEqual([1, 2], map.Keys); - Assert.AreSequenceEqual(["one", "two"], map.Values); - } + public void Enumerate_Unchanged_VisitsEveryEntry() => + MapEnumerationAssertions.UnchangedEnumerationVisitsEveryEntry(() => new InsertionOrderMap()); } diff --git a/Containers.Test/MapEnumerationAssertions.cs b/Containers.Test/MapEnumerationAssertions.cs new file mode 100644 index 0000000..d19a4a5 --- /dev/null +++ b/Containers.Test/MapEnumerationAssertions.cs @@ -0,0 +1,74 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Containers.Tests; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Enumeration checks shared by every map, which must all behave like +/// when changed during enumeration. +/// +internal static class MapEnumerationAssertions +{ + private static IDictionary Seeded(Func> create, int count) + { + IDictionary map = create(); + for (int key = 1; key <= count; key++) + { + map.Add(key, $"value {key}"); + } + + return map; + } + + internal static void RemovingDuringForeachThrows(Func> create) + { + IDictionary map = Seeded(create, 4); + + Assert.ThrowsExactly(() => + { + foreach (KeyValuePair pair in map) + { + map.Remove(pair.Key); + } + }); + } + + internal static void EveryMutatorInvalidatesEnumerators(Func> create) + { + Action>[] mutations = + [ + m => m.Add(9, "nine"), + m => m[9] = "nine", + m => m[1] = "uno", + m => m.Remove(1), + m => m.Remove(new KeyValuePair(1, "value 1")), + m => m.Clear(), + ]; + + foreach (Action> mutate in mutations) + { + IDictionary map = Seeded(create, 2); + using IEnumerator> pairs = map.GetEnumerator(); + using IEnumerator keys = map.Keys.GetEnumerator(); + using IEnumerator values = map.Values.GetEnumerator(); + Assert.IsTrue(pairs.MoveNext()); + Assert.IsTrue(keys.MoveNext()); + Assert.IsTrue(values.MoveNext()); + + mutate(map); + + Assert.ThrowsExactly(() => pairs.MoveNext()); + Assert.ThrowsExactly(() => keys.MoveNext()); + Assert.ThrowsExactly(() => values.MoveNext()); + } + } + + internal static void UnchangedEnumerationVisitsEveryEntry(Func> create) + { + IDictionary map = Seeded(create, 2); + + Assert.AreSequenceEqual([1, 2], map.Keys); + Assert.AreSequenceEqual(["value 1", "value 2"], map.Values); + } +} diff --git a/Containers.Test/OrderedMapTests.cs b/Containers.Test/OrderedMapTests.cs index 59510d0..f4ff983 100644 --- a/Containers.Test/OrderedMapTests.cs +++ b/Containers.Test/OrderedMapTests.cs @@ -519,56 +519,14 @@ public void LargeCollection_MaintainsOrder() } [TestMethod] - public void Enumerate_RemovingDuringForeach_Throws() - { - OrderedMap map = new() { [1] = "one", [2] = "two", [3] = "three", [4] = "four" }; - - Assert.ThrowsExactly(() => - { - foreach (KeyValuePair pair in map) - { - map.Remove(pair.Key); - } - }); - } + public void Enumerate_RemovingDuringForeach_Throws() => + Tests.MapEnumerationAssertions.RemovingDuringForeachThrows(() => new OrderedMap()); [TestMethod] - public void Enumerate_ChangesMadeThroughEveryMutator_Throw() - { - Action>[] mutations = - [ - m => m.Add(9, "nine"), - m => m[9] = "nine", - m => m[1] = "uno", - m => m.Remove(1), - m => m.Remove(new KeyValuePair(1, "one")), - m => m.Clear(), - ]; - - foreach (Action> mutate in mutations) - { - OrderedMap map = new() { [1] = "one", [2] = "two" }; - using IEnumerator> pairs = map.GetEnumerator(); - using IEnumerator keys = map.Keys.GetEnumerator(); - using IEnumerator values = map.Values.GetEnumerator(); - Assert.IsTrue(pairs.MoveNext()); - Assert.IsTrue(keys.MoveNext()); - Assert.IsTrue(values.MoveNext()); - - mutate(map); - - Assert.ThrowsExactly(() => pairs.MoveNext()); - Assert.ThrowsExactly(() => keys.MoveNext()); - Assert.ThrowsExactly(() => values.MoveNext()); - } - } + public void Enumerate_ChangesMadeThroughEveryMutator_Throw() => + Tests.MapEnumerationAssertions.EveryMutatorInvalidatesEnumerators(() => new OrderedMap()); [TestMethod] - public void Enumerate_Unchanged_VisitsEveryEntry() - { - OrderedMap map = new() { [1] = "one", [2] = "two" }; - - Assert.AreSequenceEqual([1, 2], map.Keys); - Assert.AreSequenceEqual(["one", "two"], map.Values); - } + public void Enumerate_Unchanged_VisitsEveryEntry() => + Tests.MapEnumerationAssertions.UnchangedEnumerationVisitsEveryEntry(() => new OrderedMap()); } diff --git a/Containers/ContiguousCollection.cs b/Containers/ContiguousCollection.cs index 4764db5..b315cf8 100644 --- a/Containers/ContiguousCollection.cs +++ b/Containers/ContiguousCollection.cs @@ -349,21 +349,17 @@ public void TrimExcess() /// /// Enumeration benefits from the contiguous memory layout with optimal cache performance. /// - public IEnumerator GetEnumerator() - { - int expected = version; - return Enumerate(); + public IEnumerator GetEnumerator() => Enumerate(version); - IEnumerator Enumerate() + private IEnumerator Enumerate(int expected) + { + for (int i = 0; i < Count; i++) { - for (int i = 0; i < Count; i++) - { - Enumeration.ThrowIfModified(expected, version); - yield return items[i]; - } - Enumeration.ThrowIfModified(expected, version); + yield return items[i]; } + + Enumeration.ThrowIfModified(expected, version); } /// diff --git a/Containers/ContiguousMap.cs b/Containers/ContiguousMap.cs index dec8f68..6f0e8b4 100644 --- a/Containers/ContiguousMap.cs +++ b/Containers/ContiguousMap.cs @@ -513,22 +513,18 @@ public bool Remove(KeyValuePair item) => /// /// Enumeration benefits from the contiguous memory layout with optimal cache performance. /// - public IEnumerator> GetEnumerator() - { - int expected = version; - return Enumerate(); + public IEnumerator> GetEnumerator() => Enumerate(version); - IEnumerator> Enumerate() + private IEnumerator> Enumerate(int expected) + { + for (int i = 0; i < Count; i++) { - for (int i = 0; i < Count; i++) - { - Entry entry = items[i]; - Enumeration.ThrowIfModified(expected, version); - yield return new KeyValuePair(entry.Key, entry.Value); - } - Enumeration.ThrowIfModified(expected, version); + Entry entry = items[i]; + yield return new KeyValuePair(entry.Key, entry.Value); } + + Enumeration.ThrowIfModified(expected, version); } /// @@ -699,21 +695,17 @@ public void CopyTo(TKey[] array, int arrayIndex) } } - public IEnumerator GetEnumerator() - { - int expected = map.version; - return Enumerate(); + public IEnumerator GetEnumerator() => Enumerate(map.version); - IEnumerator Enumerate() + private IEnumerator Enumerate(int expected) + { + for (int i = 0; i < map.Count; i++) { - for (int i = 0; i < map.Count; i++) - { - Enumeration.ThrowIfModified(expected, map.version); - yield return map.items[i].Key; - } - Enumeration.ThrowIfModified(expected, map.version); + yield return map.items[i].Key; } + + Enumeration.ThrowIfModified(expected, map.version); } IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); @@ -762,21 +754,17 @@ public void CopyTo(TValue[] array, int arrayIndex) } } - public IEnumerator GetEnumerator() - { - int expected = map.version; - return Enumerate(); + public IEnumerator GetEnumerator() => Enumerate(map.version); - IEnumerator Enumerate() + private IEnumerator Enumerate(int expected) + { + for (int i = 0; i < map.Count; i++) { - for (int i = 0; i < map.Count; i++) - { - Enumeration.ThrowIfModified(expected, map.version); - yield return map.items[i].Value; - } - Enumeration.ThrowIfModified(expected, map.version); + yield return map.items[i].Value; } + + Enumeration.ThrowIfModified(expected, map.version); } IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); diff --git a/Containers/ContiguousSet.cs b/Containers/ContiguousSet.cs index 9e7a766..947f6f5 100644 --- a/Containers/ContiguousSet.cs +++ b/Containers/ContiguousSet.cs @@ -330,21 +330,17 @@ public bool Remove(T item) /// /// Enumeration benefits from the contiguous memory layout with optimal cache performance. /// - public IEnumerator GetEnumerator() - { - int expected = version; - return Enumerate(); + public IEnumerator GetEnumerator() => Enumerate(version); - IEnumerator Enumerate() + private IEnumerator Enumerate(int expected) + { + for (int i = 0; i < Count; i++) { - for (int i = 0; i < Count; i++) - { - Enumeration.ThrowIfModified(expected, version); - yield return items[i]; - } - Enumeration.ThrowIfModified(expected, version); + yield return items[i]; } + + Enumeration.ThrowIfModified(expected, version); } /// diff --git a/Containers/InsertionOrderMap.cs b/Containers/InsertionOrderMap.cs index e09efae..71d760e 100644 --- a/Containers/InsertionOrderMap.cs +++ b/Containers/InsertionOrderMap.cs @@ -390,22 +390,18 @@ public bool Remove(KeyValuePair item) => /// Returns an enumerator that iterates through the map in insertion order. /// /// An enumerator for the map. - public IEnumerator> GetEnumerator() - { - int expected = version; - return Enumerate(); + public IEnumerator> GetEnumerator() => Enumerate(version); - IEnumerator> Enumerate() + private IEnumerator> Enumerate(int expected) + { + for (int i = 0; i < items.Count; i++) { - for (int i = 0; i < items.Count; i++) - { - Entry entry = items[i]; - Enumeration.ThrowIfModified(expected, version); - yield return new KeyValuePair(entry.Key, entry.Value); - } - Enumeration.ThrowIfModified(expected, version); + Entry entry = items[i]; + yield return new KeyValuePair(entry.Key, entry.Value); } + + Enumeration.ThrowIfModified(expected, version); } /// @@ -457,21 +453,17 @@ public void CopyTo(TKey[] array, int arrayIndex) } } - public IEnumerator GetEnumerator() - { - int expected = map.version; - return Enumerate(); + public IEnumerator GetEnumerator() => Enumerate(map.version); - IEnumerator Enumerate() + private IEnumerator Enumerate(int expected) + { + for (int i = 0; i < map.items.Count; i++) { - for (int i = 0; i < map.items.Count; i++) - { - Enumeration.ThrowIfModified(expected, map.version); - yield return map.items[i].Key; - } - Enumeration.ThrowIfModified(expected, map.version); + yield return map.items[i].Key; } + + Enumeration.ThrowIfModified(expected, map.version); } IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); @@ -511,21 +503,17 @@ public void CopyTo(TValue[] array, int arrayIndex) } } - public IEnumerator GetEnumerator() - { - int expected = map.version; - return Enumerate(); + public IEnumerator GetEnumerator() => Enumerate(map.version); - IEnumerator Enumerate() + private IEnumerator Enumerate(int expected) + { + for (int i = 0; i < map.items.Count; i++) { - for (int i = 0; i < map.items.Count; i++) - { - Enumeration.ThrowIfModified(expected, map.version); - yield return map.items[i].Value; - } - Enumeration.ThrowIfModified(expected, map.version); + yield return map.items[i].Value; } + + Enumeration.ThrowIfModified(expected, map.version); } IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); diff --git a/Containers/OrderedMap.cs b/Containers/OrderedMap.cs index c438916..0fc1b5e 100644 --- a/Containers/OrderedMap.cs +++ b/Containers/OrderedMap.cs @@ -403,22 +403,18 @@ public bool Remove(KeyValuePair item) /// Returns an enumerator that iterates through the map. /// /// An enumerator that can be used to iterate through the map. - public IEnumerator> GetEnumerator() - { - int expected = version; - return Enumerate(); + public IEnumerator> GetEnumerator() => Enumerate(version); - IEnumerator> Enumerate() + private IEnumerator> Enumerate(int expected) + { + for (int i = 0; i < items.Count; i++) { - for (int i = 0; i < items.Count; i++) - { - Entry entry = items[i]; - Enumeration.ThrowIfModified(expected, version); - yield return new KeyValuePair(entry.Key, entry.Value); - } - Enumeration.ThrowIfModified(expected, version); + Entry entry = items[i]; + yield return new KeyValuePair(entry.Key, entry.Value); } + + Enumeration.ThrowIfModified(expected, version); } /// @@ -512,21 +508,17 @@ public void CopyTo(TKey[] array, int arrayIndex) } } - public IEnumerator GetEnumerator() - { - int expected = map.version; - return Enumerate(); + public IEnumerator GetEnumerator() => Enumerate(map.version); - IEnumerator Enumerate() + private IEnumerator Enumerate(int expected) + { + for (int i = 0; i < map.items.Count; i++) { - for (int i = 0; i < map.items.Count; i++) - { - Enumeration.ThrowIfModified(expected, map.version); - yield return map.items[i].Key; - } - Enumeration.ThrowIfModified(expected, map.version); + yield return map.items[i].Key; } + + Enumeration.ThrowIfModified(expected, map.version); } IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); @@ -573,21 +565,17 @@ public void CopyTo(TValue[] array, int arrayIndex) } } - public IEnumerator GetEnumerator() - { - int expected = map.version; - return Enumerate(); + public IEnumerator GetEnumerator() => Enumerate(map.version); - IEnumerator Enumerate() + private IEnumerator Enumerate(int expected) + { + for (int i = 0; i < map.items.Count; i++) { - for (int i = 0; i < map.items.Count; i++) - { - Enumeration.ThrowIfModified(expected, map.version); - yield return map.items[i].Value; - } - Enumeration.ThrowIfModified(expected, map.version); + yield return map.items[i].Value; } + + Enumeration.ThrowIfModified(expected, map.version); } IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); From e2a87d9e93489215f5c9464e1f2be8831344a8cd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 23:53:54 +0000 Subject: [PATCH 3/3] Share the map enumerators through IVersionedEntries The three maps each carried their own versioned pair, key and value iterators. The code was identical and sat inside runs the maps already shared, so Sonar counted it as duplicated new code. Each map now exposes Version, KeyAt and ValueAt through an internal IVersionedEntries interface, and Enumeration.Pairs/Keys/Values do the iterating once. The version is still captured when GetEnumerator() is called. ContiguousMap.Clear bumps the version first rather than mid-method, so the bump does not extend the run it shares with InsertionOrderMap. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PtWVWaDh3nqJL1eKD7vFxU --- Containers/ContiguousMap.cs | 54 +++++++++------------------------ Containers/Enumeration.cs | 48 +++++++++++++++++++++++++++++ Containers/IVersionedEntries.cs | 28 +++++++++++++++++ Containers/InsertionOrderMap.cs | 52 +++++++++---------------------- Containers/OrderedMap.cs | 52 +++++++++---------------------- 5 files changed, 119 insertions(+), 115 deletions(-) create mode 100644 Containers/IVersionedEntries.cs diff --git a/Containers/ContiguousMap.cs b/Containers/ContiguousMap.cs index 6f0e8b4..7266412 100644 --- a/Containers/ContiguousMap.cs +++ b/Containers/ContiguousMap.cs @@ -46,7 +46,8 @@ namespace ktsu.Containers; )] public class ContiguousMap : IDictionary, - IReadOnlyDictionary + IReadOnlyDictionary, + IVersionedEntries where TKey : notnull { /// @@ -131,6 +132,15 @@ public override int GetHashCode() /// private int version; + /// + int IVersionedEntries.Version => version; + + /// + TKey IVersionedEntries.KeyAt(int index) => items[index].Key; + + /// + TValue IVersionedEntries.ValueAt(int index) => items[index].Value; + /// /// The internal dictionary used for fast key-based lookups to array indices. /// @@ -451,6 +461,7 @@ public bool TryGetValue(TKey key, out TValue value) /// public void Clear() { + version++; #if NET5_0_OR_GREATER if (RuntimeHelpers.IsReferenceOrContainsReferences()) #else @@ -461,7 +472,6 @@ public void Clear() Array.Clear(items, 0, Count); } Count = 0; - version++; keyToIndex.Clear(); } @@ -513,19 +523,7 @@ public bool Remove(KeyValuePair item) => /// /// Enumeration benefits from the contiguous memory layout with optimal cache performance. /// - public IEnumerator> GetEnumerator() => Enumerate(version); - - private IEnumerator> Enumerate(int expected) - { - for (int i = 0; i < Count; i++) - { - Enumeration.ThrowIfModified(expected, version); - Entry entry = items[i]; - yield return new KeyValuePair(entry.Key, entry.Value); - } - - Enumeration.ThrowIfModified(expected, version); - } + public IEnumerator> GetEnumerator() => Enumeration.Pairs(this); /// /// Returns an enumerator that iterates through the map. @@ -695,18 +693,7 @@ public void CopyTo(TKey[] array, int arrayIndex) } } - public IEnumerator GetEnumerator() => Enumerate(map.version); - - private IEnumerator Enumerate(int expected) - { - for (int i = 0; i < map.Count; i++) - { - Enumeration.ThrowIfModified(expected, map.version); - yield return map.items[i].Key; - } - - Enumeration.ThrowIfModified(expected, map.version); - } + public IEnumerator GetEnumerator() => Enumeration.Keys(map); IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } @@ -754,18 +741,7 @@ public void CopyTo(TValue[] array, int arrayIndex) } } - public IEnumerator GetEnumerator() => Enumerate(map.version); - - private IEnumerator Enumerate(int expected) - { - for (int i = 0; i < map.Count; i++) - { - Enumeration.ThrowIfModified(expected, map.version); - yield return map.items[i].Value; - } - - Enumeration.ThrowIfModified(expected, map.version); - } + public IEnumerator GetEnumerator() => Enumeration.Values(map); IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } diff --git a/Containers/Enumeration.cs b/Containers/Enumeration.cs index cbae8e4..19f6bc7 100644 --- a/Containers/Enumeration.cs +++ b/Containers/Enumeration.cs @@ -21,4 +21,52 @@ internal static void ThrowIfModified(int expected, int actual) throw new InvalidOperationException("Collection was modified; enumeration operation may not execute."); } } + + /// + /// Enumerates a map's entries, throwing if the map changes before enumeration finishes. + /// + /// The type of the keys. + /// The type of the values. + /// The map. + /// An enumerator over the entries. + internal static IEnumerator> Pairs(IVersionedEntries map) => + Iterate(map, map.Version, static (m, i) => new KeyValuePair(m.KeyAt(i), m.ValueAt(i))); + + /// + /// Enumerates a map's keys, throwing if the map changes before enumeration finishes. + /// + /// The type of the keys. + /// The type of the values. + /// The map. + /// An enumerator over the keys. + internal static IEnumerator Keys(IVersionedEntries map) => + Iterate(map, map.Version, static (m, i) => m.KeyAt(i)); + + /// + /// Enumerates a map's values, throwing if the map changes before enumeration finishes. + /// + /// The type of the keys. + /// The type of the values. + /// The map. + /// An enumerator over the values. + internal static IEnumerator Values(IVersionedEntries map) => + Iterate(map, map.Version, static (m, i) => m.ValueAt(i)); + + /// + /// The version is captured by the caller, when the enumerator is created, not on the first + /// MoveNext, so a change made in between is caught as it is by . + /// + private static IEnumerator Iterate( + IVersionedEntries map, + int expected, + Func, int, TResult> select) + { + for (int i = 0; i < map.Count; i++) + { + ThrowIfModified(expected, map.Version); + yield return select(map, i); + } + + ThrowIfModified(expected, map.Version); + } } diff --git a/Containers/IVersionedEntries.cs b/Containers/IVersionedEntries.cs new file mode 100644 index 0000000..023676b --- /dev/null +++ b/Containers/IVersionedEntries.cs @@ -0,0 +1,28 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Containers; + +/// +/// A map whose entries can be read by position and whose changes are counted, so one set of +/// enumerators in can serve every map. +/// +/// The type of the keys. +/// The type of the values. +internal interface IVersionedEntries +{ + /// Gets the number of entries. + public int Count { get; } + + /// Gets a number that changes whenever the map does. + public int Version { get; } + + /// Gets the key of the entry at a position. + /// The position. + /// The key. + public TKey KeyAt(int index); + + /// Gets the value of the entry at a position. + /// The position. + /// The value. + public TValue ValueAt(int index); +} diff --git a/Containers/InsertionOrderMap.cs b/Containers/InsertionOrderMap.cs index 71d760e..76a6d23 100644 --- a/Containers/InsertionOrderMap.cs +++ b/Containers/InsertionOrderMap.cs @@ -33,7 +33,8 @@ namespace ktsu.Containers; )] public class InsertionOrderMap : IDictionary, - IReadOnlyDictionary + IReadOnlyDictionary, + IVersionedEntries where TKey : notnull { /// @@ -55,6 +56,15 @@ private struct Entry(TKey key, TValue value) /// private int version; + /// + int IVersionedEntries.Version => version; + + /// + TKey IVersionedEntries.KeyAt(int index) => items[index].Key; + + /// + TValue IVersionedEntries.ValueAt(int index) => items[index].Value; + /// /// The internal dictionary used for fast key-based lookups. /// @@ -390,19 +400,7 @@ public bool Remove(KeyValuePair item) => /// Returns an enumerator that iterates through the map in insertion order. /// /// An enumerator for the map. - public IEnumerator> GetEnumerator() => Enumerate(version); - - private IEnumerator> Enumerate(int expected) - { - for (int i = 0; i < items.Count; i++) - { - Enumeration.ThrowIfModified(expected, version); - Entry entry = items[i]; - yield return new KeyValuePair(entry.Key, entry.Value); - } - - Enumeration.ThrowIfModified(expected, version); - } + public IEnumerator> GetEnumerator() => Enumeration.Pairs(this); /// /// Returns an enumerator that iterates through the map. @@ -453,18 +451,7 @@ public void CopyTo(TKey[] array, int arrayIndex) } } - public IEnumerator GetEnumerator() => Enumerate(map.version); - - private IEnumerator Enumerate(int expected) - { - for (int i = 0; i < map.items.Count; i++) - { - Enumeration.ThrowIfModified(expected, map.version); - yield return map.items[i].Key; - } - - Enumeration.ThrowIfModified(expected, map.version); - } + public IEnumerator GetEnumerator() => Enumeration.Keys(map); IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } @@ -503,18 +490,7 @@ public void CopyTo(TValue[] array, int arrayIndex) } } - public IEnumerator GetEnumerator() => Enumerate(map.version); - - private IEnumerator Enumerate(int expected) - { - for (int i = 0; i < map.items.Count; i++) - { - Enumeration.ThrowIfModified(expected, map.version); - yield return map.items[i].Value; - } - - Enumeration.ThrowIfModified(expected, map.version); - } + public IEnumerator GetEnumerator() => Enumeration.Values(map); IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } diff --git a/Containers/OrderedMap.cs b/Containers/OrderedMap.cs index 0fc1b5e..5f82d39 100644 --- a/Containers/OrderedMap.cs +++ b/Containers/OrderedMap.cs @@ -36,7 +36,8 @@ namespace ktsu.Containers; )] public class OrderedMap(IComparer? comparer = null) : IDictionary, - IReadOnlyDictionary + IReadOnlyDictionary, + IVersionedEntries where TKey : notnull { /// @@ -64,6 +65,15 @@ comparer is null /// private int version; + /// + int IVersionedEntries.Version => version; + + /// + TKey IVersionedEntries.KeyAt(int index) => items[index].Key; + + /// + TValue IVersionedEntries.ValueAt(int index) => items[index].Value; + /// /// The comparer used to maintain sorted order by key. /// @@ -403,19 +413,7 @@ public bool Remove(KeyValuePair item) /// Returns an enumerator that iterates through the map. /// /// An enumerator that can be used to iterate through the map. - public IEnumerator> GetEnumerator() => Enumerate(version); - - private IEnumerator> Enumerate(int expected) - { - for (int i = 0; i < items.Count; i++) - { - Enumeration.ThrowIfModified(expected, version); - Entry entry = items[i]; - yield return new KeyValuePair(entry.Key, entry.Value); - } - - Enumeration.ThrowIfModified(expected, version); - } + public IEnumerator> GetEnumerator() => Enumeration.Pairs(this); /// /// Returns an enumerator that iterates through the map. @@ -508,18 +506,7 @@ public void CopyTo(TKey[] array, int arrayIndex) } } - public IEnumerator GetEnumerator() => Enumerate(map.version); - - private IEnumerator Enumerate(int expected) - { - for (int i = 0; i < map.items.Count; i++) - { - Enumeration.ThrowIfModified(expected, map.version); - yield return map.items[i].Key; - } - - Enumeration.ThrowIfModified(expected, map.version); - } + public IEnumerator GetEnumerator() => Enumeration.Keys(map); IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } @@ -565,18 +552,7 @@ public void CopyTo(TValue[] array, int arrayIndex) } } - public IEnumerator GetEnumerator() => Enumerate(map.version); - - private IEnumerator Enumerate(int expected) - { - for (int i = 0; i < map.items.Count; i++) - { - Enumeration.ThrowIfModified(expected, map.version); - yield return map.items[i].Value; - } - - Enumeration.ThrowIfModified(expected, map.version); - } + public IEnumerator GetEnumerator() => Enumeration.Values(map); IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); }