Fix indexing in ordered subsets.#6821
Conversation
| { | ||
| int minIndex = _minIndex + count; | ||
| return minIndex >= _maxIndex ? EmptyPartition<TElement>.Instance : new OrderedPartition<TElement>(_source, minIndex, _maxIndex); | ||
| return (uint)minIndex > (uint)_maxIndex ? EmptyPartition<TElement>.Instance : new OrderedPartition<TElement>(_source, minIndex, _maxIndex); |
There was a problem hiding this comment.
Replacing >= with > is the fix for the off-by-one. Casting to (uint) is the fix for the overflow behaviour.
There was a problem hiding this comment.
Can we revisit naming these minIndexInclusive / maxIndexInclusive now? ;)
There was a problem hiding this comment.
That might not be the worse idea.
Indeed, while I've been busy with things that have kept me away from contributing and hence had the sort of ideas one only has while not working on stuff, it's occurred to me that some of these (but not this case) could be better with a _skipped and _taken, if the source is not inherently limited to being less than int.MaxValue in size. This isn't an example of that as OrderBy() can't handle larger sources anyway, but some of the others might be. I had looking at that as the next thing I wanted to look at after some Expressions tests. Maybe I should bump it, and double-checking all the other cases, up my list 😄
Fix off-by-one in OrderBy().Skip().Take(). Fixes #6819 Also fix handling if initial index after repeated Skip() calls overflows.
|
LGTM |
Fix indexing in ordered subsets.
|
LGTM |
Fix indexing in ordered subsets. Commit migrated from dotnet/corefx@ccd4043
Fix off-by-one in OrderBy().Skip().Take().
Fixes #6819
Also fix handling if initial index after repeated Skip() calls overflows.