Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix off-by-one similar to #6819 in other cases. #6843

Merged
stephentoub merged 2 commits into from
Mar 12, 2016
Merged

Fix off-by-one similar to #6819 in other cases. #6843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4be467c
Fix off-by-one similar to #6819 in other cases.
JonHanna Mar 12, 2016
8738b00
Name min/max index fields as inclusive.
JonHanna Mar 12, 2016
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
80 changes: 40 additions & 40 deletions src/System.Linq/src/System/Linq/Partition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,19 +140,19 @@ public int GetCount(bool onlyIfCheap)
internal sealed class OrderedPartition<TElement> : IPartition<TElement>
{
private readonly OrderedEnumerable<TElement> _source;
private readonly int _minIndex;
private readonly int _maxIndex;
private readonly int _minIndexInclusive;
private readonly int _maxIndexInclusive;

public OrderedPartition(OrderedEnumerable<TElement> source, int minIdx, int maxIdx)
public OrderedPartition(OrderedEnumerable<TElement> source, int minIdxInclusive, int maxIdxInclusive)
{
_source = source;
_minIndex = minIdx;
_maxIndex = maxIdx;
_minIndexInclusive = minIdxInclusive;
_maxIndexInclusive = maxIdxInclusive;
}

public IEnumerator<TElement> GetEnumerator()
{
return _source.GetEnumerator(_minIndex, _maxIndex);
return _source.GetEnumerator(_minIndexInclusive, _maxIndexInclusive);
}

IEnumerator IEnumerable.GetEnumerator()
Expand All @@ -162,26 +162,26 @@ IEnumerator IEnumerable.GetEnumerator()

public IPartition<TElement> Skip(int count)
{
int minIndex = _minIndex + count;
return (uint)minIndex > (uint)_maxIndex ? EmptyPartition<TElement>.Instance : new OrderedPartition<TElement>(_source, minIndex, _maxIndex);
int minIndex = _minIndexInclusive + count;
return (uint)minIndex > (uint)_maxIndexInclusive ? EmptyPartition<TElement>.Instance : new OrderedPartition<TElement>(_source, minIndex, _maxIndexInclusive);
}

public IPartition<TElement> Take(int count)
{
int maxIndex = _minIndex + count - 1;
if ((uint)maxIndex >= (uint)_maxIndex)
int maxIndex = _minIndexInclusive + count - 1;
if ((uint)maxIndex >= (uint)_maxIndexInclusive)
{
return this;
}

return new OrderedPartition<TElement>(_source, _minIndex, maxIndex);
return new OrderedPartition<TElement>(_source, _minIndexInclusive, maxIndex);
}

public TElement TryGetElementAt(int index, out bool found)
{
if ((uint)index <= (uint)(_maxIndex - _minIndex))
if ((uint)index <= (uint)(_maxIndexInclusive - _minIndexInclusive))
{
return _source.TryGetElementAt(index + _minIndex, out found);
return _source.TryGetElementAt(index + _minIndexInclusive, out found);
}

found = false;
Expand All @@ -190,27 +190,27 @@ public TElement TryGetElementAt(int index, out bool found)

public TElement TryGetFirst(out bool found)
{
return _source.TryGetElementAt(_minIndex, out found);
return _source.TryGetElementAt(_minIndexInclusive, out found);
}

public TElement TryGetLast(out bool found)
{
return _source.TryGetLast(_minIndex, _maxIndex, out found);
return _source.TryGetLast(_minIndexInclusive, _maxIndexInclusive, out found);
}

public TElement[] ToArray()
{
return _source.ToArray(_minIndex, _maxIndex);
return _source.ToArray(_minIndexInclusive, _maxIndexInclusive);
}

public List<TElement> ToList()
{
return _source.ToList(_minIndex, _maxIndex);
return _source.ToList(_minIndexInclusive, _maxIndexInclusive);
}

public int GetCount(bool onlyIfCheap)
{
return _source.GetCount(_minIndex, _maxIndex, onlyIfCheap);
return _source.GetCount(_minIndexInclusive, _maxIndexInclusive, onlyIfCheap);
}
}

Expand All @@ -219,8 +219,8 @@ public static partial class Enumerable
private sealed class ListPartition<TSource> : Iterator<TSource>, IPartition<TSource>
{
private readonly IList<TSource> _source;
private readonly int _minIndex;
private readonly int _maxIndex;
private readonly int _minIndexInclusive;
private readonly int _maxIndexInclusive;
private int _index;

public ListPartition(IList<TSource> source, int minIndexInclusive, int maxIndexInclusive)
Expand All @@ -229,19 +229,19 @@ public ListPartition(IList<TSource> source, int minIndexInclusive, int maxIndexI
Debug.Assert(minIndexInclusive >= 0);
Debug.Assert(minIndexInclusive <= maxIndexInclusive);
_source = source;
_minIndex = minIndexInclusive;
_maxIndex = maxIndexInclusive;
_minIndexInclusive = minIndexInclusive;
_maxIndexInclusive = maxIndexInclusive;
_index = minIndexInclusive;
}

public override Iterator<TSource> Clone()
{
return new ListPartition<TSource>(_source, _minIndex, _maxIndex);
return new ListPartition<TSource>(_source, _minIndexInclusive, _maxIndexInclusive);
}

public override bool MoveNext()
{
if ((_state == 1 & _index <= _maxIndex) && _index < _source.Count)
if ((_state == 1 & _index <= _maxIndexInclusive) && _index < _source.Count)
{
_current = _source[_index];
++_index;
Expand All @@ -254,27 +254,27 @@ public override bool MoveNext()

public override IEnumerable<TResult> Select<TResult>(Func<TSource, TResult> selector)
{
return new SelectListPartitionIterator<TSource, TResult>(_source, selector, _minIndex, _maxIndex);
return new SelectListPartitionIterator<TSource, TResult>(_source, selector, _minIndexInclusive, _maxIndexInclusive);
}

public IPartition<TSource> Skip(int count)
{
int minIndex = _minIndex + count;
return minIndex >= _maxIndex ? EmptyPartition<TSource>.Instance : new ListPartition<TSource>(_source, minIndex, _maxIndex);
int minIndex = _minIndexInclusive + count;
return (uint)minIndex > (uint)_maxIndexInclusive ? EmptyPartition<TSource>.Instance : new ListPartition<TSource>(_source, minIndex, _maxIndexInclusive);
}

public IPartition<TSource> Take(int count)
{
int maxIndex = _minIndex + count - 1;
return (uint)maxIndex >= (uint)_maxIndex ? this : new ListPartition<TSource>(_source, _minIndex, maxIndex);
int maxIndex = _minIndexInclusive + count - 1;
return (uint)maxIndex >= (uint)_maxIndexInclusive ? this : new ListPartition<TSource>(_source, _minIndexInclusive, maxIndex);
}

public TSource TryGetElementAt(int index, out bool found)
{
if ((uint)index <= (uint)(_maxIndex - _minIndex) && index < _source.Count - _minIndex)
if ((uint)index <= (uint)(_maxIndexInclusive - _minIndexInclusive) && index < _source.Count - _minIndexInclusive)
{
found = true;
return _source[_minIndex + index];
return _source[_minIndexInclusive + index];
}

found = false;
Expand All @@ -283,10 +283,10 @@ public TSource TryGetElementAt(int index, out bool found)

public TSource TryGetFirst(out bool found)
{
if (_source.Count > _minIndex)
if (_source.Count > _minIndexInclusive)
{
found = true;
return _source[_minIndex];
return _source[_minIndexInclusive];
}

found = false;
Expand All @@ -296,10 +296,10 @@ public TSource TryGetFirst(out bool found)
public TSource TryGetLast(out bool found)
{
int lastIndex = _source.Count - 1;
if (lastIndex >= _minIndex)
if (lastIndex >= _minIndexInclusive)
{
found = true;
return _source[Math.Min(lastIndex, _maxIndex)];
return _source[Math.Min(lastIndex, _maxIndexInclusive)];
}

found = false;
Expand All @@ -311,12 +311,12 @@ private int Count
get
{
int count = _source.Count;
if (count <= _minIndex)
if (count <= _minIndexInclusive)
{
return 0;
}

return Math.Min(count - 1, _maxIndex) - _minIndex + 1;
return Math.Min(count - 1, _maxIndexInclusive) - _minIndexInclusive + 1;
}
}

Expand All @@ -329,7 +329,7 @@ public TSource[] ToArray()
}

TSource[] array = new TSource[count];
for (int i = 0, curIdx = _minIndex; i != array.Length; ++i, ++curIdx)
for (int i = 0, curIdx = _minIndexInclusive; i != array.Length; ++i, ++curIdx)
{
array[i] = _source[curIdx];
}
Expand All @@ -346,8 +346,8 @@ public List<TSource> ToList()
}

List<TSource> list = new List<TSource>(count);
int end = _minIndex + count;
for (int i = _minIndex; i != end; ++i)
int end = _minIndexInclusive + count;
for (int i = _minIndexInclusive; i != end; ++i)
{
list.Add(_source[i]);
}
Expand Down
44 changes: 22 additions & 22 deletions src/System.Linq/src/System/Linq/Select.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,8 +686,8 @@ private sealed class SelectListPartitionIterator<TSource, TResult> : Iterator<TR
{
private readonly IList<TSource> _source;
private readonly Func<TSource, TResult> _selector;
private readonly int _minIndex;
private readonly int _maxIndex;
private readonly int _minIndexInclusive;
private readonly int _maxIndexInclusive;
private int _index;

public SelectListPartitionIterator(IList<TSource> source, Func<TSource, TResult> selector, int minIndexInclusive, int maxIndexInclusive)
Expand All @@ -698,19 +698,19 @@ public SelectListPartitionIterator(IList<TSource> source, Func<TSource, TResult>
Debug.Assert(minIndexInclusive <= maxIndexInclusive);
_source = source;
_selector = selector;
_minIndex = minIndexInclusive;
_maxIndex = maxIndexInclusive;
_minIndexInclusive = minIndexInclusive;
_maxIndexInclusive = maxIndexInclusive;
_index = minIndexInclusive;
}

public override Iterator<TResult> Clone()
{
return new SelectListPartitionIterator<TSource, TResult>(_source, _selector, _minIndex, _maxIndex);
return new SelectListPartitionIterator<TSource, TResult>(_source, _selector, _minIndexInclusive, _maxIndexInclusive);
}

public override bool MoveNext()
{
if ((_state == 1 & _index <= _maxIndex) && _index < _source.Count)
if ((_state == 1 & _index <= _maxIndexInclusive) && _index < _source.Count)
{
_current = _selector(_source[_index]);
++_index;
Expand All @@ -723,28 +723,28 @@ public override bool MoveNext()

public override IEnumerable<TResult2> Select<TResult2>(Func<TResult, TResult2> selector)
{
return new SelectListPartitionIterator<TSource, TResult2>(_source, CombineSelectors(_selector, selector), _minIndex, _maxIndex);
return new SelectListPartitionIterator<TSource, TResult2>(_source, CombineSelectors(_selector, selector), _minIndexInclusive, _maxIndexInclusive);
}

public IPartition<TResult> Skip(int count)
{
Debug.Assert(count > 0);
int minIndex = _minIndex + count;
return minIndex >= _maxIndex ? EmptyPartition<TResult>.Instance : new SelectListPartitionIterator<TSource, TResult>(_source, _selector, minIndex, _maxIndex);
int minIndex = _minIndexInclusive + count;
return (uint)minIndex > (uint)_maxIndexInclusive ? EmptyPartition<TResult>.Instance : new SelectListPartitionIterator<TSource, TResult>(_source, _selector, minIndex, _maxIndexInclusive);
}

public IPartition<TResult> Take(int count)
{
int maxIndex = _minIndex + count - 1;
return (uint)maxIndex >= (uint)_maxIndex ? this : new SelectListPartitionIterator<TSource, TResult>(_source, _selector, _minIndex, maxIndex);
int maxIndex = _minIndexInclusive + count - 1;
return (uint)maxIndex >= (uint)_maxIndexInclusive ? this : new SelectListPartitionIterator<TSource, TResult>(_source, _selector, _minIndexInclusive, maxIndex);
}

public TResult TryGetElementAt(int index, out bool found)
{
if ((uint)index <= (uint)(_maxIndex - _minIndex) && index < _source.Count - _minIndex)
if ((uint)index <= (uint)(_maxIndexInclusive - _minIndexInclusive) && index < _source.Count - _minIndexInclusive)
{
found = true;
return _selector(_source[_minIndex + index]);
return _selector(_source[_minIndexInclusive + index]);
}

found = false;
Expand All @@ -753,10 +753,10 @@ public TResult TryGetElementAt(int index, out bool found)

public TResult TryGetFirst(out bool found)
{
if (_source.Count > _minIndex)
if (_source.Count > _minIndexInclusive)
{
found = true;
return _selector(_source[_minIndex]);
return _selector(_source[_minIndexInclusive]);
}

found = false;
Expand All @@ -766,10 +766,10 @@ public TResult TryGetFirst(out bool found)
public TResult TryGetLast(out bool found)
{
int lastIndex = _source.Count - 1;
if (lastIndex >= _minIndex)
if (lastIndex >= _minIndexInclusive)
{
found = true;
return _selector(_source[Math.Min(lastIndex, _maxIndex)]);
return _selector(_source[Math.Min(lastIndex, _maxIndexInclusive)]);
}

found = false;
Expand All @@ -781,12 +781,12 @@ private int Count
get
{
int count = _source.Count;
if (count <= _minIndex)
if (count <= _minIndexInclusive)
{
return 0;
}

return Math.Min(count - 1, _maxIndex) - _minIndex + 1;
return Math.Min(count - 1, _maxIndexInclusive) - _minIndexInclusive + 1;
}
}

Expand All @@ -799,7 +799,7 @@ public TResult[] ToArray()
}

TResult[] array = new TResult[count];
for (int i = 0, curIdx = _minIndex; i != array.Length; ++i, ++curIdx)
for (int i = 0, curIdx = _minIndexInclusive; i != array.Length; ++i, ++curIdx)
{
array[i] = _selector(_source[curIdx]);
}
Expand All @@ -816,8 +816,8 @@ public List<TResult> ToList()
}

List<TResult> list = new List<TResult>(count);
int end = _minIndex + count;
for (int i = _minIndex; i != end; ++i)
int end = _minIndexInclusive + count;
for (int i = _minIndexInclusive; i != end; ++i)
{
list.Add(_selector(_source[i]));
}
Expand Down
9 changes: 9 additions & 0 deletions src/System.Linq/tests/OrderedSubsetting.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,15 @@ public void Count()
Assert.Equal(1, Enumerable.Range(0, 100).Shuffle().OrderBy(i => i).Take(2).Skip(1).Count());
}

[Fact]
public void SkipTakesOnlyOne()
{
Assert.Equal(new[] { 1 }, Enumerable.Range(1, 10).Shuffle().OrderBy(i => i).Take(1));
Assert.Equal(new[] { 2 }, Enumerable.Range(1, 10).Shuffle().OrderBy(i => i).Skip(1).Take(1));
Assert.Equal(new[] { 3 }, Enumerable.Range(1, 10).Shuffle().OrderBy(i => i).Take(3).Skip(2));
Assert.Equal(new[] { 1 }, Enumerable.Range(1, 10).Shuffle().OrderBy(i => i).Take(3).Take(1));
}

[Fact]
public void EmptyToArray()
{
Expand Down
9 changes: 9 additions & 0 deletions src/System.Linq/tests/RangeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,15 @@ public void SkipExcessive()
Assert.Empty(Enumerable.Range(10, 10).Skip(20));
}

[Fact]
public void SkipTakeCanOnlyBeOne()
{
Assert.Equal(new[] { 1 }, Enumerable.Range(1, 10).Take(1));
Assert.Equal(new[] { 2 }, Enumerable.Range(1, 10).Skip(1).Take(1));
Assert.Equal(new[] { 3 }, Enumerable.Range(1, 10).Take(3).Skip(2));
Assert.Equal(new[] { 1 }, Enumerable.Range(1, 10).Take(3).Take(1));
}

[Fact]
public void ElementAt()
{
Expand Down
Loading