What's wrong
The doc comment on OrderedCollection<T>.IndexOf (Containers/OrderedCollection.cs) promises "the zero-based index of the first occurrence". Remove is also documented as removing the first occurrence. Both are built on BinarySearch, which returns as soon as comparer.Compare(items[mid], item) == 0. With duplicate keys, that is whichever equal element the midpoint lands on, not the leftmost one.
Reproduction (confirmed with a temporary MSTest against current main)
OrderedCollection<int> a = [1, 1, 1];
a.IndexOf(1); // returns 1, expected 0
OrderedCollection<int> b = [0, 2, 2, 2, 2, 3];
b.IndexOf(2); // returns 2, expected 1
Why it matters
OrderedCollection explicitly allows duplicates, so callers rely on the documented "first occurrence". Some examples of what goes wrong:
- A caller that iterates from
IndexOf(x) to find every equal element misses the ones before the returned index.
IndexOf(x) == 0 checks give the wrong answer.
- With a key-based
IComparer<T> (for example, sorting records by priority), Remove(item) can remove a different record with the same key than the one passed in.
Suggested fix
Add a lower-bound search and use it in IndexOf and Remove. When the comparison is 0, record mid and keep searching left (right = mid - 1), then return the leftmost match, or -1/~left when nothing matched. BinarySearch can keep its current "any match" contract if that is intended for insertion, but its doc comment should then say so.
For Remove under a key-based comparer, consider scanning the equal-key run for the element that Equals the argument, so the removed element is the one passed in.
Acceptance criteria
IndexOf(1) on [1,1,1] returns 0, and IndexOf(2) on [0,2,2,2,2,3] returns 1.
Remove removes the first equal element, as documented.
- Regression tests cover duplicate runs of odd and even length.
What's wrong
The doc comment on
OrderedCollection<T>.IndexOf(Containers/OrderedCollection.cs) promises "the zero-based index of the first occurrence".Removeis also documented as removing the first occurrence. Both are built onBinarySearch, which returns as soon ascomparer.Compare(items[mid], item) == 0. With duplicate keys, that is whichever equal element the midpoint lands on, not the leftmost one.Reproduction (confirmed with a temporary MSTest against current
main)Why it matters
OrderedCollectionexplicitly allows duplicates, so callers rely on the documented "first occurrence". Some examples of what goes wrong:IndexOf(x)to find every equal element misses the ones before the returned index.IndexOf(x) == 0checks give the wrong answer.IComparer<T>(for example, sorting records by priority),Remove(item)can remove a different record with the same key than the one passed in.Suggested fix
Add a lower-bound search and use it in
IndexOfandRemove. When the comparison is 0, recordmidand keep searching left (right = mid - 1), then return the leftmost match, or-1/~leftwhen nothing matched.BinarySearchcan keep its current "any match" contract if that is intended for insertion, but its doc comment should then say so.For
Removeunder a key-based comparer, consider scanning the equal-key run for the element thatEqualsthe argument, so the removed element is the one passed in.Acceptance criteria
IndexOf(1)on[1,1,1]returns 0, andIndexOf(2)on[0,2,2,2,2,3]returns 1.Removeremoves the first equal element, as documented.