Move range

[Scharps]

Overload implementation and tests for and overload of Move that takes a range as a parameter. #27

Overload just uses existing implementation of Move but takes range as parameter instead

[viceroypenguin]

Consider the following desire:

Enumerable.Range(1, 10)
  .Select(x => x.ToString())
  .Move(^5..^2, 10);

What would be the expected sequence? What is your code returning? Also consider what would happen if to was changed to an Index, such that it was .Move(^5..^2, ^0).

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (bcecba2) 93.30% compared to head (12e4d6b) 93.03%.
Report is 9 commits behind head on master.

❗ Current head 12e4d6b differs from pull request most recent head 3f48c9a. Consider uploading reports for the commit 3f48c9a to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #584      +/-   ##
==========================================
- Coverage   93.30%   93.03%   -0.27%     
==========================================
  Files         242      255      +13     
  Lines        7767     8141     +374     
  Branches     1596     1618      +22     
==========================================
+ Hits         7247     7574     +327     
- Misses        329      358      +29     
- Partials      191      209      +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Scharps]

Consider the following desire:

Enumerable.Range(1, 10)
  .Select(x => x.ToString())
  .Move(^5..^2, 10);

What would be the expected sequence? What is your code returning? Also consider what would happen if to was changed to an Index, such that it was .Move(^5..^2, ^0).

Can deferred execution still be maintained if the 'from end' operator is used?

I have tried to avoid it by checking each index:

var length = 0;
if (range.Start.IsFromEnd || range.End.IsFromEnd || toIndex.IsFromEnd)
{
	length = source.GetCollectionCount();
}
var fromIndex = range.Start.IsFromEnd ? length - range.Start.Value : range.Start.Value;
var count = (range.End.IsFromEnd ? length - range.End.Value : range.End.Value) - fromIndex;
var to = toIndex.IsFromEnd ? length - toIndex.Value : toIndex.Value;
return source.Move
(
	fromIndex,
	count,
	to
);

I think that it may mislead the user if they decide to go for the overload rather than the original.

[viceroypenguin]

Take a look at the Take(Range) and the ElementAt(Index) operators for how this would be implemented properly in a deferred execution environment. Specifically:

• if all of the values are not IsFromEnd, then you can pass the values directly
• if the source.TryGetCollectionCount returns a value, you can convert all of the values to simple values using index.GetOffset(count)
• Otherwise, some more complicated buffering and analysis will be required, covering the following cases:

start.IsFromEnd	end.IsFromEnd	to.IsFromEnd
false	false	true
false	true	false
true	false	false
false	true	true
true	false	true
true	true	false
true	true	true

NB: in some of these cases, there are corner cases to consider, such as if the start is before the end, due to the length of the sequence, etc.

[viceroypenguin]

I'm sorry, but I think you may still be missing the point about how ^ indexes are actually interpreted. Please review the TakeRangeFromEndIterator method for the complexities involved here.

A simple call to the existing Move method is only valid for the case where all three values are !.IsFromEnd. A separate iterator method will be required to handle any other case.

[viceroypenguin]

if TryGetCollectionCount() fails to return a value, then GetCollectionCount() will throw an exception. try calling Enumerable.Range(1, 10).Select(Identity).GetCollectionCount().

[viceroypenguin]

Review https://github.com/viceroypenguin/SuperLinq/blob/master/Source/SuperLinq/Take.cs#L87-L92 for why you cannot do this here.

[Scharps]

Take a look at the Take(Range) and the ElementAt(Index) operators for how this would be implemented properly in a deferred execution environment. Specifically:

• if all of the values are not IsFromEnd, then you can pass the values directly
• if the source.TryGetCollectionCount returns a value, you can convert all of the values to simple values using index.GetOffset(count)
• Otherwise, some more complicated buffering and analysis will be required, covering the following cases:

start.IsFromEnd end.IsFromEnd to.IsFromEnd
false false true
false true false
true false false
false true true
true false true
true true false
true true true
NB: in some of these cases, there are corner cases to consider, such as if the start is before the end, due to the length of the sequence, etc.

Apologies for the silence,

I'm struggling to find a use for a buffer. In the TakeFromEndIterator it makes sense since the length to take is known and the distance from the end is also known (0).

For example, in the Move case:

start.IsFromEnd	end.IsFromEnd	to.IsFromEnd
true	true	false

We can infer the length of the buffer from the difference between the start and end range. The buffer can be added to when enumerating through, but since the number of elements is unknown, the values could be dropped from the buffer once it has reached capacity.

If there is a different between start.IsFromEnd and end.IsFromEnd the length of the buffer cannot be inferred.

[viceroypenguin]

If there is a different between start.IsFromEnd and end.IsFromEnd the length of the buffer cannot be inferred.

This is correct. That's why a buffer must be used in at least some of these cases.

If both are IsFromEnd, then you can compare the values, and if end.Value > start.Value, then you can return an empty sequence (because there can never be any elements between start and end if end is before start). But if start < end, then you need to buffer start.Value items so that when the sequence ends, you have the final items available from which to select the items necessary.

[Scharps]

If both are IsFromEnd, then you can compare the values, and if end.Value > start.Value, then you can return an empty sequence (because there can never be any elements between start and end if end is before start). But if start < end, then you need to buffer start.Value items so that when the sequence ends, you have the final items available from which to select the items necessary.

Why would moving an invalid range result in an empty sequence? Also, I believe that this scenario can also arise from differences in IsFromEnd in the range.

I am having difficulty understanding how there could be an approach that would differ from the original implementation. Finding the number of elements seems to be required for all approaches that has parameters with IsFromEnd.

Scenarios

1.

start.IsFromEnd	end.IsFromEnd	to.IsFromEnd
false	false	false

Original method can be used without enumerating to find count.

2.

start.IsFromEnd	end.IsFromEnd	to.IsFromEnd
false	false	true

Cannot yield any elements until count is known. It is possible that elements could be moved to start of the IEnumerable.

For example, Move(1, 3, ^5). If the number of elements is equal to 5.

3.

start.IsFromEnd	end.IsFromEnd	to.IsFromEnd
false	true	false

Cannot yield any elements until count is known. It's possible that the range's end occurs before the start. Yielding any value here would infringe on returning an empty sequence if end index < start index.

4.

start.IsFromEnd	end.IsFromEnd	to.IsFromEnd
false	true	true

Combination of 2 and 3.

5.

start.IsFromEnd	end.IsFromEnd	to.IsFromEnd
true	false	false

Same as 3.

6.

start.IsFromEnd	end.IsFromEnd	to.IsFromEnd
true	false	true

Same as 4.

7.

start.IsFromEnd	end.IsFromEnd	to.IsFromEnd
true	true	false

Cannot yield until count is known. Elements at start may be moved.

8.

start.IsFromEnd	end.IsFromEnd	to.IsFromEnd
true	true	true

Count must be known.

---

Please let me know if I'm missing something, but my current understanding is that finding the count and relying on the existing Move implementation is the only way.

[viceroypenguin]

Consider the following: [0 1 2 3 4 5 6 7 8 9].Move(3..4, ^3)

  v
[0 1 2 3 4 5 6 7 8 9]
buffer: [0]
move: []
returned: []

    v
[0 1 2 3 4 5 6 7 8 9]
buffer: [0 1]
returned: []

      v
[0 1 2 3 4 5 6 7 8 9]
buffer: [0 1 2]
returned: []

        v
[0 1 2 3 4 5 6 7 8 9]
buffer: [1 2 3]
move: []
returned: [0]

          v
[0 1 2 3 4 5 6 7 8 9]
buffer: [2 3 4]
move: []
returned: [0 1]

            v
[0 1 2 3 4 5 6 7 8 9]
buffer: [3 4 5]
move: []
returned: [0 1 2]

              v
[0 1 2 3 4 5 6 7 8 9]
buffer: [4 5 6]
move: [3]
returned: [0 1 2]

                v
[0 1 2 3 4 5 6 7 8 9]
buffer: [5 6 7]
move: [3]
returned: [0 1 2 4]

                  v
[0 1 2 3 4 5 6 7 8 9]
buffer: [6 7 8]
move: [3]
returned: [0 1 2 4 5]

                    v
[0 1 2 3 4 5 6 7 8 9]
buffer: [7 8 9]
move: [3]
returned: [0 1 2 4 5 6]

                    v
[0 1 2 3 4 5 6 7 8 9]
buffer: [7 8 9]
move: []
returned: [0 1 2 4 5 6 3]

                    v
[0 1 2 3 4 5 6 7 8 9]
buffer: []
move: []
returned: [0 1 2 4 5 6 3 7 8 9]

// note this algorithm still works in your case of Move(1..3, ^5),
// because it queues up 5 items before dumping,
// so there's a chance to interrupt the sequence at any point.

In this case, the entire sequence did not need to be buffered, only the ^3 and the move amount. Similar evaluations can be done when all three are IsFromEnd - just buffer up the max value of the three, and dump anything that exceeds the queue length. Once processed, you can evaluate the ^ in the context of what's left. The other cases are more tricky, but essentially variations of this theme.

[tacosontitan]

There are some legibility issues with this IMHO.

[Scharps]

In this case, the entire sequence did not need to be buffered, only the ^3 and the move amount. Similar evaluations can be done when all three are IsFromEnd - just buffer up the max value of the three, and dump anything that exceeds the queue length. Once processed, you can evaluate the ^ in the context of what's left. The other cases are more tricky, but essentially variations of this theme.

I've added an additional buffer for the case that the range has moved to a prior position in the enumerable. A more elegant solution is required, but there seems to be common themes between the two solved scenarios so far.