views:

302

answers:

3

I am reading this blog: Pipes and filters pattern

I am confused by this code snippet:

public class Pipeline<T>
{
    private readonly List<IOperation<T>> operations = new List<IOperation<T>>();

    public Pipeline<T> Register(IOperation<T> operation)
    {
        operations.Add(operation);
        return this;
    }

    public void Execute()
    {
        IEnumerable<T> current = new List<T>();
        foreach (IOperation<T> operation in operations)
        {
            current = operation.Execute(current);
        }
        IEnumerator<T> enumerator = current.GetEnumerator();
        while (enumerator.MoveNext());
    }
}

what is the purpose of this statement: while (enumerator.MoveNext());? seems this code is a noop.

+1  A: 
while (enumerator.MoveNext());

Inside the current block of code, there is no affect (it moves through all the items in the enumeration). The displayed code doesn't act on the current element in the enumeration. What might be happening is that the MoveNext() method is moving to the next element, and it is doing something to the objects in the collection (updating an internal value, pull the next from the database etc.). Since the type is List<T> this is probably not the case, but in other instances it could be.

Kevin
so, the enclosing *while* code does not make a difference?
Patrick Karcher
`while(enumerator.MoveNext());` is a complete loop. This effectively traverses the entire enumerable.
Anthony Pegram
+7  A: 

First consider this:

IEnumerable<T> current = new List<T>();
foreach (IOperation<T> operation in operations)
{
    current = operation.Execute(current);
}

This code appears to be creating nested enumerables, each of which takes elements from the previous, applies some operation to them, and passes the result to the next. But it only constructs the enumerables. Nothing actually happens yet. It's just ready to go, stored in the variable current. There are lots of ways to implement IOperation.Execute but it could be something like this.

IEnumerable<T> Execute(IEnumerable<T> ts)
{
    foreach (T t in ts)
        yield return this.operation(t); // Perform some operation on t.
}

Another option suggested in the article is a sort:

IEnumerable<T> Execute(IEnumerable<T> ts)
{
    // Thank-you LINQ!
    // This was 10 lines of non-LINQ code in the original article.
    return ts.OrderBy(t => t.Foo);
}

Now look at this:

IEnumerator<T> enumerator = current.GetEnumerator();
while (enumerator.MoveNext());

This actually causes the chain of operations to be performed. When the elements are requested from the enumeration, it causes elements from the original enumerable to be passed through the chain of IOperations, each of which performs some operation on them. The end result is discarded so only the side-effect of the operation is interesting - such as writing to the console or logging to a file. This would have been a simpler way to write the last two lines:

foreach (T t in current) {}

Another thing to observe is that the initial list that starts the process is an empty list so for this to make sense some instances of T have to be created inside the first operation. In the article this is done by asking the user for input from the console.

Mark Byers
what about useing .ToList() to make the operations to be performed
Benny
@Benny: Yes, you could also write `current.ToList();` instead of the last two lines. This would also cause the enumeration to occur.
Mark Byers
I suspect that this whole thing can only work because there is somewhere a `yield return` embedded in the `Execute` method of the `operations` list. So what I'm saying is, that this cannot be possibly done without the use of `yield return`, because only that causes lazy evaluation. Am I right?
kahoon
To me, this simply looks like bad design. Calling MoveNext() should not have as a side effect that "the missile was launched". There should not be a side effect. MoveNext() should do what it promises to do as part of its contract - and nothing else!
Seventh Element
Since all IEnumerator<T>'s are IDisposable; the Dispose method should be called whenever an IEnumerator is no longer in use. Since the foreach construct does this automatically, I prefer the empty bodied foreach.
devgeezer
+3  A: 

In this case, the while (enumerator.MoveNext()); is simply evaluating all the items that are returned by the final IOperation<T>. It looks a little confusing, but the empty List<T> is only created in order to supply a value to the first IOperation<T>.

In many collections this would do exaclty nothing as you suggest, but given that we are talking about the pipes and filters pattern it is likely that the final value is some sort of iterator that will cause code to be executed. It could be something like this, for example (assuming that is an integer):

public class WriteToConsoleOperation : IOperation<int>
{
    public IEnumerable<int> Execute(IEnumerable<int> ints)
    {
        foreach (var i in ints)
        {
            Console.WriteLine(i);
            yield return i;
        }
     }
}

So calling MoveNext() for each item on the IEnumerator<int> returned by this iterator will return each of the values (which are ignored in the while loop) but also output each of the values to the console.

Does that make sense?

Damian Powell
sure it make sense.
Benny