views:

132

answers:

4

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?

+2  A: 

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.

Adam Robinson
Adam is right. When I first started using enumerables, I used `IEnumerator<T>` directly as you are doing. It took a bit of a mental shift to center the code around `IEnumerable<T>` instead, but the resulting code is much more composable. Consider LINQ to Objects, which uses `IEnumerable<T>` in a composable fashion.
Stephen Cleary
The implementer needs the ability to get the current and decide if they can act upon it and move on to the next item. Any implemented method will be able to consume zero or more of the items allowing execution to continue on the remaining set. Similar to popping off Queue<T>, but 1) they cannot enqueue any items, and 2) I don't want to have to copy all of the items into a Queue<T> as they are potentially supplied via lazy execution as needed.
McKAMEY
@StephenCleary, how would LINQ handle the situation I just described?
McKAMEY
@McKAMEY: Could you post a small code sample of how this sort of arrangement works? Is this just a small list of functions that get called with the same enumerator, meaning that it could have already completed enumerating the values (`MoveNext()` returning `false`) before all functions have been called?
Adam Robinson
Basically it is a recursive descent parser taking a sequence of tokens with the ability to extend it with custom grammars at a couple key places. So the main processing internally moves through a single loop of the `IEnumerable<T>` while passing to subsequent methods based upon the current state.
McKAMEY
Ah, the old RDP! :) You may be interested in my `EnumeratorWrapper<T>` class defined [here](http://nitokitchensink.codeplex.com/SourceControl/changeset/view/54782#1131464). If you have an LL(1) grammar, that should be the only time you'd have to deal with `IEnumerator<T>` directly. Sample usage (for a CSV lexer) is [here](http://nitokitchensink.codeplex.com/SourceControl/changeset/view/54782#1131429).
Stephen Cleary
Uh... isn't `EnumeratorWrapper` that what I'm doing?! If I'm "not supposed" to be passing around an `IEnumerator<T>` then why would I build a data structure based off of it? I'm a little confused at the mixed advice. Seems like the main thing it provides is a mechanism for the receiver to know what the last value of `MoveNext` was. But in my case, if `MoveNext` returned false then I wouldn't pass it to any other methods. I'd be done.
McKAMEY
+2  A: 

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.

Drew Noakes
+1  A: 

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

Dan Bryant
Yes, that is exactly what I want: some sort of generic Stream<T>. I need to be able to effectively see the next item (IEnumerator.Current -> Peek) and consume it (IEnumerator.MoveNext -> Pop).
McKAMEY
Marking this as the answer since it is the summary of what I need to do. Ideally, this would share the answer with Jon Skeet's answer as that demonstrated a core flaw in my approach (ValueType IEnumerators), but it lacked a suggestion of a solution.
McKAMEY
+6  A: 

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.

Jon Skeet
So basically what I'm hearing is that IEnumerator<T> would cause the implementers to be required to be more cautious than I can probably expect them to be.Other than the criticism of the approach do you have a suggestion of a better approach? What are your thoughts on the Stream<T> idea or know of a class which has this functionality?
McKAMEY
Are you basically fooling the compiler into creating a boxed copy so that it's mutating the copy? Mutable value types are scary...
Dan Bryant
I'm just using the `IEnumerator<T>` to walk the `IEnumerable<T>`. I'm not fooling anyone. :)
McKAMEY
@McKAMEY, the Enumerator for most built-in collections is actually a value type, but the compiler is doing some tap-dancing so that it doesn't get boxed under typical usage. The concern here is that the state could be copied and now you're unknowingly mutating a copy of the enumerator.
Dan Bryant
Excellent point!
McKAMEY