8

I'm basicly trying to make my class able to iterate using foreach. I read this tutorial. MSDN. It seems very straight forward. However, I have a problem when I want to iterate second time. I debugged it; and it turned out that it doesn't call the Reset() function.

Class A

class A : IEnumerable, IEnumerator
{
    int[] data = { 0, 1, 2, 3, 4 };

    int position = -1;

    public object Current
    {
        get
        {
            return data[position];
        }
    }

    public bool MoveNext()
    {
        position++;
        return (position < data.Length);
    }

    public void Reset()
    {
        position = -1;
    }

    public IEnumerator GetEnumerator()
    {
        return (IEnumerator)this;
    }
}

When I run the following main function; it never calls Reset() function. So, after one loop I never be able to iterate my class again.

Main

static void Main(string[] args)
{
    A a = new A();

    foreach (var item in a)
    {
        Console.WriteLine(item);
    }

    Console.WriteLine("--- First foreach finished. ---");

    foreach (var item in a)
    {
        Console.WriteLine(item);
    }
}

Output:

0
1
2
3
4
--- First foreach finished. ---
Press any key to continue . . .

Any thoughts?

Sait
  • 19,045
  • 18
  • 72
  • 99
  • 1
    Look at this [answer](http://stackoverflow.com/a/3948450/461968) – Paul Phillips Jul 13 '12 at 17:26
  • Btw unless you are doing it for fun r a very specific reason, you should never have to implement an iterator this way. Look into "iterator blocks" aka "yield return". – Marc Gravell Jul 13 '12 at 17:34
  • I know it is old but I just found this post with google. I had the same problem and I implemented the MoveNext method in such a way that it call Reset when it comes to return with false. – dvjanm Oct 20 '13 at 07:56

4 Answers4

17

Each time foreach is called, it asks for a new IEnumerator. Returning your class instance is a bad idea - you should make a separate class to implement the IEnumerator, and return it instead.

This is often done by using a nested (private) class, and returning an instance of it. You can pass the class A instance to the private class (giving it access to data), and put the position field in that class. It would allow more than one enumerator to be created simulatenously, and will work properly with subsequent foreach calls.

For example, to modify your code, you'd do something like:

using System;
using System.Collections;

class A : IEnumerable
{
    int[] data = { 0, 1, 2, 3, 4 };

    public IEnumerator GetEnumerator()
    {
        return new AEnumerator(this);
    }

    private class AEnumerator : IEnumerator
    {
        public AEnumerator(A inst)
        {
            this.instance = inst;
        }

        private A instance;
        private int position = -1;

        public object Current
        {
            get
            {
                return instance.data[position];
            }
        }

        public bool MoveNext()
        {
            position++;
            return (position < instance.data.Length);
        }

        public void Reset()
        {
            position = -1;
        }

    }
}

Note that you can also just return the array's enumerator directly (though I was assuming you were trying to learn how to make clean enumerators):

class A : IEnumerable
{
    int[] data = { 0, 1, 2, 3, 4 };

    public IEnumerator GetEnumerator()
    {
        return data.GetEnumerator();
    }
}

Finally, you can use iterators to implement this in a far simpler manner:

class A : IEnumerable
{
    int[] data = { 0, 1, 2, 3, 4 };

    public IEnumerator GetEnumerator()
    {
        for (int i=0;i<data.Length;++i)
           yield return data[i];
    }
}

That being said, I would strongly recommend implementing IEnumerable<int> in addition to IEnumerable. Generics make this far nicer in terms of usage.

Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
  • Yes you are right. I was trying to learn how to make clean enumerators. My real code is more complicated. – Sait Jul 13 '12 at 17:34
  • 1
    @zagy Look at iterators - they're typically the nicest, practical way to implement enumerators. That being said, I'd also always implement `IEnumerable` in addition to of `IEnumerable`... – Reed Copsey Jul 13 '12 at 17:34
7

Reset() was basically a mistake. There's already a known method to get a clean enumerator if possible: GetEnumerator().

It is a requirement in the specification that iterator block implementations (for the iterator) throw an exception for this method, hence in the general case it is formally known that it can't be expected to work, and simply: nobody ever calls it. Frankly, they should also have marked it [Obsolete] on the API !

Additionally, many sequences are non-repeatable. Think of iterators sat on a NetworkStream or a random numer generator. Because iterators (in the general case) are not required to be repeatable, you should aim, where possible, to iterate them at most once. Perhaps buffering via ToList() if that is not possible.

"foreach" does not involve Reset() at any point. Just GetEnumerator(), MoveNext(), Current and Dispose().

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • Where in the specification is the requirement? I find "the implementer can simply throw a NotSupportedException." - this is not a "must". https://learn.microsoft.com/en-us/dotnet/api/system.collections.ienumerator.reset?view=netframework-4.5.2 – jifb Oct 04 '22 at 16:29
  • 1
    @jifb I should have been more specific, and said "C# language specification" when talking about "iterator block implementations", in particular ECMA 334 v6 section 14.14.5.1: "Enumerator objects do not support the IEnumerator.Reset method. Invoking this method causes a System.NotSupportedException to be thrown."; notice that I said this in a specific context: "iterator block implementations" - iterator block here referring to C# that uses `yield return` / `yield break` constructs - not just any/all random iterator implementations – Marc Gravell Oct 05 '22 at 10:51
5

The enumeration doesn't call Reset. You need to create a new instance of an enumerator, which will likely mean creating a separate class for the enumerator (i.e., not using the same type for the IEnumerable and IEnumerator), like in the code below:

public class StackOverflow_11475328
{
    class A : IEnumerable
    {
        int[] data = { 0, 1, 2, 3, 4 };

        public IEnumerator GetEnumerator()
        {
            return new AEnumerator(this);
        }

        class AEnumerator : IEnumerator
        {
            private A parent;
            private int position = -1;

            public AEnumerator(A parent)
            {
                this.parent = parent;
            }

            public object Current
            {
                get { return parent.data[position]; }
            }

            public bool MoveNext()
            {
                position++;
                return (position < parent.data.Length);
            }

            public void Reset()
            {
                position = -1;
            }
        }
    }
    public static void Test()
    {
        A a = new A();

        foreach (var item in a)
        {
            Console.WriteLine(item);
        }

        Console.WriteLine("--- First foreach finished. ---");

        foreach (var item in a)
        {
            Console.WriteLine(item);
        }
    }
}
carlosfigueira
  • 85,035
  • 14
  • 131
  • 171
0

Both Reed and Carlos are correct. Here is one way you can do this, since int[] implements IEnumerable and IEnumerable<int>

class A : IEnumerable
{ 
    int[] data = { 0, 1, 2, 3, 4 }; 

    public IEnumerator GetEnumerator() 
    { 
        return data.GetEnumerator(); 
    } 
} 

Or, to be more strongly typed, you can use the generic form of IEnumerable:

class A : IEnumerable<int>
{
    int[] data = { 0, 1, 2, 3, 4 };

    public IEnumerator<int> GetEnumerator()
    {
        return ((IEnumerable<int>)data).GetEnumerator();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return data.GetEnumerator();
    }
}
Monroe Thomas
  • 4,962
  • 1
  • 17
  • 21