Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Simplified Position API #1990

Merged
merged 1 commit into from
Dec 20, 2017
Merged

Simplified Position API #1990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions samples/LowAllocationWebServer/LowAllocationWebServerLibrary/BufferSequence.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ public bool TryGet(ref Position position, out Memory<byte> item, bool advance =
return false;
}

var sequence = position.GetItem<BufferSequence>();
item = sequence.Memory.Slice(0, sequence._written);
if (advance) { position = Position.Create(sequence._next); }
var (buffer, index) = position.Get<BufferSequence>();
item = buffer.Memory.Slice(index, buffer._written - index);
if (advance) { position = Position.Create(buffer._next); }
return true;
}

Expand All @@ -69,9 +69,9 @@ public bool TryGet(ref Position position, out ReadOnlyMemory<byte> item, bool ad
return false;
}

var sequence = position.GetItem<BufferSequence>();
item = sequence.WrittenMemory;
if (advance) { position = Position.Create(sequence._next); }
var (buffer, index) = position.Get<BufferSequence>();
item = buffer.WrittenMemory.Slice(index);
if (advance) { position = Position.Create(buffer._next); }
return true;
}

Expand Down
6 changes: 3 additions & 3 deletions src/System.Buffers.Experimental/System/Buffers/Sequences/MemoryList.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ public bool TryGet(ref Position position, out Memory<byte> item, bool advance =
return false;
}

var sequence = position.GetItem<MemoryList>();
item = sequence._data;
if (advance) { position = Position.Create(sequence._next); }
var (list, index) = position.Get<MemoryList>();
item = list._data.Slice(index);
if (advance) { position = Position.Create(list._next); }
return true;
}

Expand Down
18 changes: 9 additions & 9 deletions src/System.Buffers.Experimental/System/Buffers/Sequences/ReadOnlyBytes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ public ReadOnlyBytes Slice(Position position)
switch (kind)
{
case Type.Array:
var array = position.GetItem<byte[]>();
return new ReadOnlyBytes(array, position.Index, array.Length - position.Index);
var (array, index) = position.Get<byte[]>();
return new ReadOnlyBytes(array, index, array.Length - index);
case Type.MemoryList:
return Slice(position, Position.Create((IMemoryList<byte>)_end, _endIndex));
default: throw new NotImplementedException();
Expand All @@ -154,12 +154,12 @@ public ReadOnlyBytes Slice(Position start, Position end)
switch (kind)
{
case Type.Array:
var startArray = start.GetItem<byte[]>();
return new ReadOnlyBytes(startArray, start.Index, end.Index - start.Index);
var (array, index) = start.Get<byte[]>();
return new ReadOnlyBytes(array, index, (int)end - index);
case Type.MemoryList:
var startList = start.GetItem<IMemoryList<byte>>();
var endList = end.GetItem<IMemoryList<byte>>();
return new ReadOnlyBytes(startList, start.Index, endList, end.Index);
var (startList, startIndex) = start.Get<IMemoryList<byte>>();
var (endList, endIndex) = end.Get<IMemoryList<byte>>();
return new ReadOnlyBytes(startList, startIndex, endList, endIndex);
default:
throw new NotImplementedException();
}
Expand Down Expand Up @@ -281,8 +281,8 @@ public bool TryGet(ref Position position, out ReadOnlyMemory<byte> item, bool ad
var array = _start as byte[];
if (array != null)
{
var start = position.Index;
var length = _endIndex - position.Index;
var start = (int)position;
var length = _endIndex - (int)position;
item = new ReadOnlyMemory<byte>(array, start, length);
if (advance) position = default;
return true;
Expand Down
18 changes: 9 additions & 9 deletions src/System.Buffers.Experimental/System/Buffers/Sequences/ReadWriteBytes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ public ReadWriteBytes Slice(Position position)
switch (kind)
{
case Type.Array:
var array = position.GetItem<byte[]>();
return new ReadWriteBytes(array, position.Index, array.Length - position.Index);
var (array, index) = position.Get<byte[]>();
return new ReadWriteBytes(array, index, array.Length - index);
case Type.MemoryList:
return Slice(position, Position.Create((IMemoryList<byte>)_end, _endIndex));
default: throw new NotImplementedException();
Expand All @@ -150,12 +150,12 @@ public ReadWriteBytes Slice(Position start, Position end)
switch (kind)
{
case Type.Array:
var startArray = start.GetItem<byte[]>();
return new ReadWriteBytes(startArray, start.Index, end.Index - start.Index);
var (array, index) = start.Get<byte[]>();
return new ReadWriteBytes(array, index, (int)end - index);
case Type.MemoryList:
var startList = start.GetItem<IMemoryList<byte>>();
var endList = end.GetItem<IMemoryList<byte>>();
return new ReadWriteBytes(startList, start.Index, endList, end.Index);
var (startList, startIndex) = start.Get<IMemoryList<byte>>();
var (endList, endIndex) = end.Get<IMemoryList<byte>>();
return new ReadWriteBytes(startList, startIndex, endList, endIndex);
default:
throw new NotImplementedException();
}
Expand Down Expand Up @@ -278,8 +278,8 @@ public bool TryGet(ref Position position, out Memory<byte> item, bool advance =
var array = _start as byte[];
if (array != null)
{
var start = _startIndex + position.Index;
var length = _endIndex - _startIndex - position.Index;
var start = _startIndex + (int)position;
var length = _endIndex - _startIndex - (int)position;
item = new Memory<byte>(array, start, length);
if (advance) position = default;
return true;
Expand Down
17 changes: 5 additions & 12 deletions src/System.Buffers.Experimental/System/Buffers/Text/BufferReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ public bool TryReadBytes(out ReadOnlyBytes bytes, byte delimiter)
return false;
}

var startObj = range.Start.GetItem<object>();
var endObj = range.End.GetItem<object>();
var (startObj, startIndex) = range.Start.Get<object>();
var (endObj, endIndex) = range.End.Get<object>();
// TODO: this is a hack. Once we move this to System.Memory, we should remove
if (startObj == null || endObj == null)
{
Expand Down Expand Up @@ -302,14 +302,7 @@ public bool TryRead(out int value, bool littleEndian = false)

ReadOnlySpan<byte> Unread => _currentSpan.Slice(_currentSpanIndex);

Position Position
{
get {
var result = _currentSegmentPosition;
result += _currentSpanIndex;
return result;
}
}
Position Position =>_currentSegmentPosition + _currentSpanIndex;

Position? AdvanceToDelimiter(byte value)
{
Expand Down Expand Up @@ -358,10 +351,10 @@ Position Position
if(index != -1 && unread.Length - index < value.Length)
{
Span<byte> temp = stackalloc byte[value.Length];
int copied = Sequence.Copy(_bytes, _currentSegmentPosition + _currentSpanIndex + index, temp);
int copied = Sequence.Copy(_bytes, _currentSegmentPosition + (_currentSpanIndex + index), temp);
if (copied < value.Length) return null;

if (temp.SequenceEqual(value)) return _currentSegmentPosition + _currentSpanIndex + index;
if (temp.SequenceEqual(value)) return _currentSegmentPosition + (_currentSpanIndex + index);
else throw new NotImplementedException(); // need to check farther in this span
}

Expand Down
28 changes: 16 additions & 12 deletions src/System.Collections.Sequences/System/Collections/Sequences/Position.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,19 @@ public static Position Create<T>(T item, int index = 0) where T : class

public static implicit operator Position(int index) => new Position(index, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need all these operators, Get<T>, and static Create method?
Can we just expose item and index and add a single public constructor that takes them?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also in all of the usages of Get<T> you never check that item is not null. Do we need to add some kind of validation that position was of an expected type?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's how it was originally, but you guys wanted to make it harder for people to get access to the object. Get/Create allow you to store an internal type in object to make it impossible to retrieve by third party code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can always do Get<object> and get around it. And ReadOnlyBuffer stores publicly available types there. I don't think we can protect it from retrieving the item.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I wanted to change the check to only return the object if typeof(T) == _item.GetType().
I was planing to do it in a subsequent PR.


public static explicit operator int(Position position) => position.Index;

public int Index => _index;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public T GetItem<T>() => _item == null ? default : (T)_item;
public static explicit operator int(Position position) => position._index;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public (T item, int index) Get<T>() => (GetItem<T>(), (int)this);
public (T item, int index) Get<T>() {
var item = _item == null ? default : (T) _item;
return (item, _index);
}

public static bool operator ==(Position left, Position right) => left.Index == right.Index && left._item == right._item;
public static bool operator !=(Position left, Position right) => left.Index != right.Index || left._item != right._item;
public static bool operator ==(Position left, Position right) => left._index == right._index && left._item == right._item;
public static bool operator !=(Position left, Position right) => left._index != right._index || left._item != right._item;

public static Position operator +(Position position, int offset) => new Position(position.Index + offset, position._item);
public static Position operator +(Position value, int index)
=> new Position(value._index + index, value._item);

[EditorBrowsable(EditorBrowsableState.Never)]
public bool Equals(Position position) => this == position;
Expand All @@ -40,16 +39,21 @@ public override bool Equals(object obj) =>

[EditorBrowsable(EditorBrowsableState.Never)]
public override int GetHashCode() =>
Index.GetHashCode() ^ (_item == null ? 0 : _item.GetHashCode());
_index.GetHashCode() ^ (_item == null ? 0 : _item.GetHashCode());

[EditorBrowsable(EditorBrowsableState.Never)]
public override string ToString() =>
this==default ? "(default)" : _item == null ? $"{Index}" : $"{Index}, {_item}";
this==default ? "(default)" : _item == null ? $"{_index}" : $"{_index}, {_item}";

private Position(int index, object item)
{
_item = item;
_index = index;
}
private Position(int index)
{
_item = null;
_index = index;
}
}
}
2 changes: 1 addition & 1 deletion src/System.Collections.Sequences/System/Collections/Sequences/ResizableArray.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public bool TryGet(ref Position position, out T item, bool advance = true)
int index = (int)position;
if (index < _count) {
item = _array[index];
if (advance) { position += 1; }
if (advance) { position +=1; }
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions tests/System.Collections.Sequences.Tests/SampleCollections/LinkedContainer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ public bool TryGet(ref Position position, out T item, bool advance = true)
return false;
}

var node = position.GetItem<Node>();
if (node == null) {
var (node, index) = position.Get<Node>();
if (node == null || index != 0) {
item = default;
position = default;
return false;
Expand Down