Loop + Take for repeatedly taking elements from the same collection

Question

I sometimes need to enumerate the same collection multiple times so I wrote an extension that does this for me. I call it Loop. It takes one optional parameter startAt which allows it to start at an offset.

---

The extension works with enumerators. If it could not MoveNext then it gets a new enumerator and tries again. If the second try failed then the collection is empty, otherwise it starts from the beginning. It uses a local function that handles the enumerator by resetting it if necessary.

public static IEnumerable<T> Loop<T>(this IEnumerable<T> values, int startAt = 0)
{
    if (values == null)
    {
        throw new ArgumentNullException(nameof(values));
    }

    var moves = 0;

    // ReSharper disable once PossibleMultipleEnumeration
    var enumerator = values.GetEnumerator();

    try
    {
        while (true)
        {
            if (TryMoveNext(enumerator, out enumerator))
            {
                moves++;

                if (startAt > 0 && moves <= startAt)
                {
                    continue;
                }

                yield return enumerator.Current;
            }
            else
            {
                yield break;
            }
        }
    }
    finally
    {
        enumerator.Dispose();
    }

    bool TryMoveNext(IEnumerator<T> currentEnumerator, out IEnumerator<T> newEnumerator)
    {
        if (currentEnumerator.MoveNext())
        {
            newEnumerator = currentEnumerator;
            return true;
        }
        else
        {
            // Get a new enumerator because we took all elements and try again.

            currentEnumerator.Dispose();

            // ReSharper disable once PossibleMultipleEnumeration
            newEnumerator = values.GetEnumerator();

            // If we couldn't move after reset then we're done trying because the collection is empty.
            return newEnumerator.MoveNext();
        }
    }
}

I have the feeling like I might have overcomplicated the implementation.

---

Here are the tests I wrote for it:

[TestClass]
public class enumerableTest
{
    [TestMethod]
    public void Loop_EmptyCollection_NoneTakes()
    {
        var numbers = new int[0];
        Assert.IsFalse(numbers.Loop().Take(2).Any());
    }

    [TestMethod]
    public void Loop_LessThanAvailable_NoLoop()
    {
        var numbers = new[] { 1, 2, 3 };
        Assert.That.Collection().AreEqual(new[] { 1, 2 }, numbers.Loop().Take(2));
    }

    [TestMethod]
    public void Loop_MoreThanAvailable_OneLoop()
    {
        var numbers = new[] { 1, 2, 3 };
        Assert.That.Collection().AreEqual(new[] { 1, 2, 3, 1 }, numbers.Loop().Take(4));
    }

    [TestMethod]
    public void Loop_TwiceTheAvailable_TwoLoops()
    {
        var numbers = new[] { 1, 2, 3 };
        Assert.That.Collection().AreEqual(new[] { 1, 2, 3, 1, 2, 3, 1, 2 }, numbers.Loop().Take(8));
    }

    [TestMethod]
    public void Loop_MoreThanAvailableWithStartAt_OneLoop()
    {
        var numbers = new[] { 1, 2, 3 };
        Assert.That.Collection().AreEqual(new[] { 2, 3, 1, 2, 3, 1 }, numbers.Loop(1).Take(6));
    }

}

Answer 1

Code optimization

This can be shortened:

while (true)
{
    if (TryMoveNext(enumerator, out enumerator))
    {
        moves++;

        if (startAt > 0 && moves <= startAt)
        {
                continue;
        }

        yield return enumerator.Current;
    }
    else
    {
        yield break;
    }
}

To:

while (TryMoveNext(enumerator, out enumerator))
{
    moves++;

    if (startAt > 0 && moves <= startAt)
    {
        continue;
    }

    yield return enumerator.Current;
}

You added the if for "boolean breaking", but at the same time you've subverted the inherent "boolean breaking" that a while loop provides.

---

Enumeration needed?

Since you are looking to repeat the same collection multiple times, this collection is cyclic in nature. Whenever I think about cyclic data, the modulo operator always comes to mind. It's by far the most common use case for the modulo operator.

According to this SO answer, though there may be a minor difference, the performance difference between enumeration and indexed arrays is reasonably negligible.
Keeping that in mind, I would advocate using a modulo operator simply because it keeps the code cleaner.

Note: The linked answer does not address re-enumeration, which is relevant for your code. But tbh, I'm not sure if re-enumeration is cached (thus not costing any meaningful overhead) or not. I'm going to continue my assumption that the performance differences are too small to be considered relevant for your current use case.

public static IEnumerable<T> Loop<T>(this IEnumerable<T> values, int startPosition = 0)
{
    if (values == null)
    {
        throw new ArgumentNullException(nameof(values));
    }

    T[] array = values.ToArray();
    int count = values.Count();

    while (true)
    {
        yield return array[startPosition++ % count];
    }
}

I kept the example terse (e.g. by inlining startPosition++), but I hope it highlights the reduced complexity of the code in general.

A slight improvement would be to not let startPosition grow beyond the array's size:

    while (true)
    {
        startPosition = (++startPosition) % count;
        yield return array[startPosition];
    }

Though I think this is only really useful if you expect to enumerate up to a point of integer overflow, which I doubt is the case here.

---

Footnote

I'm torn about the startPosition variable name. Once we start using it as a counter, its name is no longer correct (it's the current position). However, I do think it fits as a parameter name.
We can assign the value to a variable with a better name; but I'm not sure if it's really that necessary for such a terse piece of code.

I can see arguments for either case. I'm unable to make a conclusive decision on whether it's necessary. I'm open to feedback on this.

If you do want to make the name more appropriate, I would suggest the following:

public static IEnumerable<T> Loop<T>(this IEnumerable<T> values, int? startPosition = null)
{
     //...

     int counter = startPosition ?? 0;

     //...
}