Question

I have a C# class which needs to process a sequence of items (IEnumerable<T>) across a bunch of methods, so I cannot simply foreach inside a method. I call .GetEnumerator() and pass this IEnumerator<T> around and it works great giving me the flexibility I need while looping through a single sequence.

Now I want to allow others to add logic into this process. The most natural way to do this is give them an interface with a method that accepts the IEnumerator<T>. Easy, done, and it works.

But I'm concerned that this is an anti-pattern. They have to know that the IEnumerator<T> has already had .MoveNext() called, so they can simply access .Current. Plus I don't see any precedent for using IEnumerator<T> in interfaces to be implemented.

  1. What pitfalls am I not considering?
  2. Is there another pattern which will allow me this same efficient mechanism (i.e. I don't want multiple copies being created/destroyed) without exposing the IEnumerator<T> itself?

Update: As I mentioned in a comment below: What I want is some sort of generic Stream<T>. I need to be able to effectively see the next item (IEnumerator.Current -> .Peek()) and consume it (IEnumerator<T>.MoveNext() -> .Pop()).

I used IEnumerator<T> because it fit the bill interface wise. I prefer to use common BCL types when they fit, but it seemed like I was abusing this one.

So question 3) Is there a class which fits this need? Or should I just create my own Stream which lazily executes the IEnumerator<T> internally? Then it would be entirely encapsulated. I'd like to not use many of the existing collections as they have internal storage, whereas I'd like the storage to be the IEnumerable<T> iteslf.


OK it sounds like the consensus is that do to IEnumerator<T> often being a ValueType as well as not knowing a priori the state of the IEnumerator<T>, that it is generally a bad idea to pass it around.

The best suggestion I've heard is to create my own class which gets passed around. Any other suggestions?

Was it helpful?

Solution

If I understand this correctly, you have a number of methods that can all call MoveNext on the sequence and you want these methods to cooperate with each-other, so you pass around an IEnumerator<T>. There's definitely some tight coupling here, as you mentioned, since you expect the enumerator to be in a particular state at the entrance to each method. It sounds like what you're really after here is something like the Stream class, which is both a collection (sort of) and an iterator (has a notion of Current location). I would wrap your iteration and any other state you need in your own class and have the various methods as members of that class

OTHER TIPS

You should definitely not pass the IEnumerator<T> around. Apart from anything else, it could have some very bizarre effects in some cases. What would you expect to happen here, for example?

using System;
using System.Collections.Generic;

class Test
{
    static void ShowCurrentAndNext(IEnumerator<int> iterator)        
    {
        Console.WriteLine("ShowCurrentAndNext");
        Console.WriteLine(iterator.Current);
        iterator.MoveNext(); // Let's assume it returns true
        Console.WriteLine(iterator.Current);
    }

    static void Main()
    {
        List<int> list = new List<int> { 1, 2, 3, 4, 5 };
        using (var iterator = list.GetEnumerator())
        {
            iterator.MoveNext(); // Get things going
            ShowCurrentAndNext(iterator);
            ShowCurrentAndNext(iterator);
            ShowCurrentAndNext(iterator);
        }
    }
}

A couple of changes to try:

using (List<int>.Enumerator iterator = list.GetEnumerator())

and

using (IEnumerator<int> iterator = list.GetEnumerator())

Try to predict the results in each case :)

Now admittedly that's a particularly evil example, but it does demonstrate some corner cases associated with passing around mutable state. I would strongly encourage you to perform all your iteration in a "central" method which calls into appropriate other methods just with the current value.

I would strongly advise against passing the enumerator itself around; what reason do you have for this, aside from needing the current value?

Unless I'm missing something obvious, I would recommend having your utility functions simply take the type that you're enumerating as a parameter, then have a single outer foreach loop that handles the actual enumeration.

Perhaps you can provide some additional information as to why you've made this design decision so far.

Sounds to me like you might benefit from using an event so that you can push notification of items to be processed out to listeners. Regular .NET events are handled in the order they're subscribed, so you might go for a more explicit approach if ordering is required.

You may also like to look at the Reactive Framework.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top