CODEWITHSUNDEEP
c#ienumerableenumerate
virtualmic
virtualmic

Reputation: 3213

Why doesn't the following work? (IEnumerable/ IEnumerator)

I was trying out the example on this MSDN page. I tried to change the GetEnumerator method. I know something doesn't seem right in that, but it complies and then doesn't run. Error is that the Enumerator has not started and that MoveNext should be called, but it is being called!

class Program
{
    static void Main(string[] args)
    { 
        foreach (var day in new DaysOfTheWekk())
        {
            Console.WriteLine(day) ;
        }
        Console.ReadLine();
    }
}

public class DaysOfTheWekk: IEnumerable
{
    private string[] days = {"Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"};

    public IEnumerator GetEnumerator()
    {
        days.GetEnumerator().MoveNext();
        yield return days.GetEnumerator().Current;
    }
}

Upvotes: 3

Views: 5015

Answers (5)

Fırat Çağlar Akbulut
Fırat Çağlar Akbulut

Reputation: 428

Another solution is, you should ensure that you return always the same Iterator when you call method GetEnumerator(). This is the sample implementation:

public class DaysOfTheWeek : IEnumerable
{
    private string[] days = { "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat" };
    private IEnumerator iterator;

    public DaysOfTheWeek()
    {
        iterator = days.GetEnumerator();
        iterator.MoveNext();
    }

    public IEnumerator GetEnumerator()
    {
        return iterator;
    }
}

Calling MoveNext() in the constructor is not necessary, you have to call iterator.MoveNext() method before iterator.current.

The point is you will always use the same iterator when you call GetEnumerator() method.

Upvotes: 1

Rune FS
Rune FS

Reputation: 21752

You have called MoveNext() on a different Enumerator

your code is equivalent to

public IEnumerator GetEnumerator()
{
    var enumerator1 = days.GetEnumerator();
    enumerator1.MoveNext();
    var enumerator2 = days.GetEnumerator();
    yield return enumerator2.Current;
}

each time you call GetEnumerator() a new enumerator is constrcuted (at least for BCL implementations of IEnumerable) and as you can see from the code above you construct two enumerators and call MoveNext on one and Current on the other. This is a key conceptual difference between properties and methods. A method should be expected to return the result of an operation while a property should be expected to return the same value unless the state of the object changed. There also seems to be a logical bug in your code, you are only returning the first element and it would fail if there's none, so essentially you've implemented the .Single() method your code would work if you changed to 

public IEnumerator GetEnumerator()
{
    var enumerator = days.GetEnumerator();
    while(enumerator.MoveNext()){
       yield return enumerator.Current;
    }
}

which of course is functionaly the same as

public IEnumerator GetEnumerator()
{
    foreach(var day in days){
       yield return day;
    }
}

Upvotes: 4

sloth
sloth

Reputation: 101142

days.GetEnumerator().MoveNext();
yield return days.GetEnumerator().Current;

Here you create two different enumerators. You call MoveNext on the first, then you create another one in the line below and access Current on that.

Upvotes: 1

usr
usr

Reputation: 171246

I think you assumed that days.GetEnumerator() always returned the same enumerator. It returns a fresh one each time for a good reason - if there was only one, different pieces of code were unable to enumerate at the same time. It would not compose well.

Call days.GetEnumerator() once, or write return days.GetEnumerator();.

Upvotes: 0

Matten
Matten

Reputation: 17603

Why do you want to call moveNext? Just leave out the .Current:

public class DaysOfTheWeek: IEnumerable
{
    private string[] days = {"Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"};

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

Otherwise use a while loop since:

public class DaysOfTheWeek: IEnumerable
{
    private string[] days = {"Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"};

    public IEnumerator GetEnumerator()
    {
        var enumerator = days.GetEnumerator();
        while(enumerator.MoveNext())
        { 
            yield return enumerator.Current;
        }
    }
}

Explanation: The GetEnumerator() method always returns a new enumerator, so if you call GetEnumerator().Current, then the MoveNext() function has not been called on the newly returned instance! Use a variable as stated in my second example, instead.

Upvotes: 4

Related Questions