Make OrderedCollection.IndexOf and Remove find the first occurrence - #64
Merged
matt-edmondson merged 1 commit intoSep 26, 2026
Merged
Conversation
…nce [patch] IndexOf and Remove were built on BinarySearch, which returns whichever equal element the midpoint lands on. With duplicates, IndexOf(1) on [1,1,1] returned 1 rather than the documented first occurrence, and Remove could drop a different element with the same key under a key-based comparer. Both now use a lower-bound search. Remove prefers the element in the equal-key run that also Equals the argument, falling back to the first. BinarySearch keeps its any-match contract for insertion, and its doc now says so. Fixes #62 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHW69XTeuuLbfVdQdJ8uQh
|
This was referenced Sep 26, 2026
matt-edmondson
deleted the
claude/containers-62-indexof-first-occurrence
branch
September 26, 2026 09:52
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #62
Problem
OrderedCollection<T>.IndexOfandRemoveare documented to act on the first occurrence. Both usedBinarySearch, which returns whichever equal element the midpoint hits. So[1,1,1].IndexOf(1)returned 1,[0,2,2,2,2,3].IndexOf(2)returned 2, and under a key-based comparerRemovecould drop a different record with the same key.Fix
FindFirstdoes a lower-bound binary search: on a match it recordsmidand keeps searching left.IndexOfusesFindFirst.Removestarts atFindFirst, removes the first element in the equal-key run that alsoEqualsthe argument, and falls back to the first element in the run when none does.BinarySearchkeeps its existing "any match" behaviour, whichAddrelies on for insertion. Its XML doc now says that.Tests
New tests in
OrderedCollectionTests:IndexOf_OddLengthDuplicateRun_ReturnsFirstOccurrence([1,1,1]gives 0)IndexOf_EvenLengthDuplicateRun_ReturnsFirstOccurrence([0,2,2,2,2,3]gives 1)Remove_KeyComparerWithDuplicateKeys_RemovesTheElementPassedInRemove_KeyComparerWithNoExactMatch_RemovesFirstOccurrenceWith the fix reverted, all 4 fail. With it, the full suite passes (377/377).
This PR is independent of #63; both branch from
main.🤖 Generated with Claude Code
https://claude.ai/code/session_01UHW69XTeuuLbfVdQdJ8uQh
Generated by Claude Code