7
\$\begingroup\$

Funny thing, the following test works:

    [TestMethod]
    public void Iterate_Once()
    {
        var x = new[] { 1, 2, 3, 4, 5, 6 }.ToIterable();
        CollectionAssert.AreEqual(new[] { 1, 2 }, x.Take(2).ToArray());
        CollectionAssert.AreEqual(new[] { 3, 4, 5 }, x.Take(3).ToArray());
    }

Where:

static class Iterable
{
    public static IEnumerable<T> ToIterable<T>(this IEnumerable<T> source)
    {
        var it = source.ToList().GetEnumerator();
        return once();

        IEnumerable<T> once()
        {
            while (it.MoveNext())
                yield return it.Current;
        }
    }
}

That should be safe to do not dispose an iterator of in-memory collection.

Please do not put me in jail for that :)

\$\endgroup\$
  • 1
    \$\begingroup\$ IMO this is more of a theoretical than a review question, but could you please explain your assertion "that should be safe to do not dispose of in-memory collection" - at least for me? \$\endgroup\$ – Henrik Hansen Oct 12 at 5:49
  • 1
    \$\begingroup\$ @HenrikHansen We use List<T> iterator here which is IDisposable. I bet that this concrete Dispose() implementation does nothing, so it should be safe to do not invoke it. \$\endgroup\$ – Dmitry Nogin Oct 12 at 5:57
  • 2
    \$\begingroup\$ OK, betting is a strange kind of development paradigm and relying on an implementation detail is likewise probably not a good idea :-). BtW: the meaning of x.Take(3) (after calling x.Take(2)) is somewhat different than the normal behavior on this kind of IEnumerable<T> return by ToIterable(). Further will for instance x.Take(3); x.Any(i => i == 1); return false for Any(), and many other IEnumerable<T> linq-apis will behave unexpectedly on this kind of IEnumerable<T>. IMO you're mixing two distinct concepts that should be kept distinct. \$\endgroup\$ – Henrik Hansen Oct 12 at 7:51
  • \$\begingroup\$ Why would you do this and not just use ToArray or ToList? \$\endgroup\$ – CharlesNRice Oct 12 at 12:27
  • 2
    \$\begingroup\$ I see what you doing now. You want something like obserable.publish \$\endgroup\$ – CharlesNRice Oct 12 at 14:07
1
\$\begingroup\$

So what you are trying to do is to have an implementation of IEnumerable<T> which has a singleton IEnumerator<T>? Why not implement that explicitly, instead of using undocumented (?) behaviour of List<T>'s iterator?

Simply create a class that consumes a IEnumerable<T>, takes its iterator and wraps it in an implementation of IEnumerator<T> that has a dysfunctional Reset method. IEnumerator<T>.GetEnumerator() can then return the instance to that same iterator instead of creating a new one for each call. It could look something like this:

  public class OnlyOnceIterator<T> : IEnumerable<T>, IEnumerator<T> {
    private readonly IEnumerator<T> enumerator;
    internal OnlyOnceIterator(IEnumerable<T> sequence) {
      enumerator = sequence.GetEnumerator();
    }

    public T Current => enumerator.Current;

    object IEnumerator.Current => enumerator.Current;

    public void Dispose() {
      enumerator.Dispose();
    }

    public IEnumerator<T> GetEnumerator() => this;

    public bool MoveNext() {
      return enumerator.MoveNext();
    }

    public void Reset() {
      return;
    }

    IEnumerator IEnumerable.GetEnumerator() => this;
  }

This could then be used in an extension method:

  public static OnlyOnceIterator<T> ToOnlyOnceIterator<T>(this IEnumerable<T> sequence) {
    if (sequence == null) {
      throw new System.ArgumentNullException(nameof(sequence));
    }

    return new OnlyOnceIterator<T>(sequence);
  }

Additionally, I would prefer a more specific return type than IEnumerable<T>, since the returned behaviour is significantly different to what any user might expect from IEnumerable<T>. Consider creating a new interface that inherits from IEnumerable<T> with a more descriptive name. Through inheritance, Linq will still be able to be used, and you can create methods that only accept this specific interface, (compare this to IOrderedEnumerable<T>).

\$\endgroup\$
  • 1
    \$\begingroup\$ I bet it will not work with EF as you would dispose original decorated iterator on the very first run. \$\endgroup\$ – Dmitry Nogin Oct 12 at 16:30
  • \$\begingroup\$ The points you make are all valid, but I also believe @DmitryNogin is right on this one. \$\endgroup\$ – Denis Oct 12 at 17:41
  • \$\begingroup\$ @DmitryNogin You might be right, I have no experience with EF. Maybe changing the implementation of OnlyOnceIterator.Dispose to not dispose the enumerable (I overlooked that) might work. OTOH, in your answer you are eagerly evaluating your input sequence, so if you immediately consume that, that would mitigate the disposing risk as well. \$\endgroup\$ – JAD Oct 12 at 17:47
0
\$\begingroup\$

I think the following would be the reasonable approach to implement it. Please note that To prefix in ToIterable documents full sequence materialization as framework design guideline naming conventions dictate - compare to As prefix.

It would be interesting to figure out how AsIterable could be done :)

public static class Iterable
{
    public static IEnumerable<T> ToIterable<T>(this IEnumerable<T> source)
    {
        var array = source.ToArray();
        var i = 0;
        IEnumerable<T> once()
        {
            while (i < array.Length)
                yield return array[i++];
        }

        return once();
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.