35

Basically I have an anonymous method that I use for my BackgroundWorker:

worker.DoWork += ( sender, e ) =>
{
    foreach ( var effect in GlobalGraph.Effects )
    {
        // Returns EffectResult
        yield return image.Apply (effect);
    }
};

When I do this the compiler tells me:

"The yield statement cannot be used inside an anonymous method or lambda expression"

So in this case, what's the most elegant way to do this? Btw this DoWork method is inside a static method, in case that matters for the solution.

Joan Venge
  • 315,713
  • 212
  • 479
  • 689
  • Is the background worker producing the `image` or is it populating the `GlobalGraph.Effects` enumerable? – Enigmativity Mar 23 '11 at 22:04
  • Yes the BW is producing the image but the EffectResult has status about the effect, not image data or anything like that. – Joan Venge Mar 23 '11 at 22:05
  • possible duplicate of [In C#, why can't an anonymous method contain a yield statement?](http://stackoverflow.com/questions/1217729/in-c-why-cant-an-anonymous-method-contain-a-yield-statement) – Mauricio Scheffer Jan 15 '12 at 01:01

8 Answers8

17

Unfortunately you can't.

The compiler does not allow you to combine the two "magic" pieces of code. Both involve rewriting your code to support what you want to do:

  1. An anonymous method is done by moving the code to a proper method, and lifting local variables to fields on the class with that method
  2. An iterator method is rewritten as a state machine

You can, however, rewrite the code to return the collection, so in your particular case I would do this:

worker.DoWork += ( sender, e ) =>
{
    return GlobalGraph.Effects
        .Select(effect => image.Apply(effect));
};

though it looks odd for an event (sender, e) to return anything at all. Are you sure you're showing a real scenario for us?


Edit Ok, I think I see what you're trying to do here.

You have a static method call, and then you want to execute code in the background, and then return data from that static method once the background call completes.

This is, while possible, not a good solution since you're effectively pausing one thread to wait for another, that was started directly before you paused the thread. In other words, all you're doing is adding overhead of context switching.

Instead you need to just kick off the background work, and then when that work is completed, process the resulting data.

Lasse V. Karlsen
  • 380,855
  • 102
  • 628
  • 825
  • Thanks Lasse. Actually you are right, I am not sure if I am doing the right thing on this. Basically this worker is inside a static method called Run which is supposed to return IEnumerable. How should I do it? Outside the DoWork? I just did it there because applying a bunch of effects is all this method is doing (and returning the results). – Joan Venge Mar 23 '11 at 21:24
  • Also even though I showed it like this, I have a for loop doing something else before image.Apply is called, for each iteration, can I put a for loop inside a lambda expression? – Joan Venge Mar 23 '11 at 21:37
  • You can't do a `return` inside a lambda that is meant to return from the enclosing method. A `return` in a lambda returns from the lambda itself. – Enigmativity Mar 23 '11 at 22:01
  • @Enigmativity There is no way to return from the enclosing method *at all* in an anonymous method. That's like saying that method A calls method B and method B returns from method A, it's not possible. It is, however, a sign that there is something wrong with the whole line of thinking in this question. – Lasse V. Karlsen Mar 24 '11 at 08:57
  • Basically I am trying to get the results one by one not when the static method is finished its work. Is that what you meant? – Joan Venge Mar 24 '11 at 17:00
10

Perhaps just return the linq expression and defer execution like yield:

return GlobalGraph.Effects.Select(x => image.Apply(x));
Tejs
  • 40,736
  • 10
  • 68
  • 86
5

Unless I'm missing something, you can't do what you're asking.

(I do have an answer for you, so please read past my explanation of why you can't do what you're doing first, and then read on.)

You full method would look something like this:

public static IEnumerable<EffectResult> GetSomeValues()
{
    // code to set up worker etc
    worker.DoWork += ( sender, e ) =>
    {
        foreach ( var effect in GlobalGraph.Effects )
        {
            // Returns EffectResult
            yield return image.Apply (effect);
        }
    };
}

If we assume that your code was "legal" then when GetSomeValues is called, even though the DoWork handler is added to worker, the lambda expression isn't executed until the DoWork event is fired. So the call to GetSomeValues completes without returning any results and the lamdba may or may not get called at a later stage - which is then too late for the caller of the GetSomeValues method anyway.

Your best answer is to the use Rx.

Rx turns IEnumerable<T> on its head. Instead of requesting values from an enumerable, Rx has values pushed to you from an IObservable<T>.

Since you're using a background worker and responding to an event you are effectively having the values pushed to you already. With Rx it becomes easy to do what you're trying to do.

You have a couple of options. Probably the simplest is to do this:

public static IObservable<IEnumerable<EffectResult>> GetSomeValues()
{
    // code to set up worker etc
    return from e in Observable.FromEvent<DoWorkEventArgs>(worker, "DoWork")
           select (
               from effect in GlobalGraph.Effects
               select image.Apply(effect)
           );
}

Now callers of your GetSomeValues method would do this:

GetSomeValues().Subscribe(ers =>
{
    foreach (var er in ers)
    {
        // process each er
    }
});

If you know that DoWork is only going to fire once, then this approach might be a little better:

public static IObservable<EffectResult> GetSomeValues()
{
    // code to set up worker etc
    return Observable
        .FromEvent<DoWorkEventArgs>(worker, "DoWork")
        .Take(1)
        .Select(effect => from effect in GlobalGraph.Effects.ToObservable()
                          select image.Apply(effect))
        .Switch();  
}

This code looks a little more complicated, but it just turns a single do work event into a stream of EffectResult objects.

Then the calling code looks like this:

GetSomeValues().Subscribe(er =>
{
    // process each er
});

Rx can even be used to replace the background worker. This might be the best option for you:

public static IObservable<EffectResult> GetSomeValues()
{
    // set up code etc
    return Observable
        .Start(() => from effect in GlobalGraph.Effects.ToObservable()
                     select image.Apply(effect), Scheduler.ThreadPool)
        .Switch();  
}

The calling code is the same as the previous example. The Scheduler.ThreadPool tells Rx how to "schedule" the processing of subscriptions to the observer.

I hope this helps.

Enigmativity
  • 113,464
  • 11
  • 89
  • 172
  • Thanks that seems like a good solution. Do you also know a solution purely using the BW? I would like to use Rx, but currently I don't want to add in new dependencies, if possible. Later I can work to beautify the code. – Joan Venge Mar 23 '11 at 22:04
  • @Joan - You can't use the BW to do what you want. It doesn't make sense to use it. Your calling code is getting an `IEnumerable` which it has to block on while the BW is doing its thing. You might as well avoid the BW altogether because of that. You need to use something like Rx or the TPL. – Enigmativity Mar 23 '11 at 22:10
  • OK then how am I supposed to update the UI while using BW? It seems like it must be trivial to do this at least. I am using WPF and gradually want to update the collection the UI is binded to so that's why I wanted to use `IEnumerable`. – Joan Venge Mar 23 '11 at 22:13
  • @Joan - Running thru any `IEnumerable` is a blocking call. Using a `yield return` doesn't make it asynchronous. Your BW should do all of the data processing in the background and then, once complete, hand over the processed data to the UI in one step. You're trying to hand it over in many steps (ie using an enumerable) and that's not going to work. Try using TPL. If you're on .NET 3.5 SP1 then you can get the TPL by installing Rx. It's bundled in there. – Enigmativity Mar 23 '11 at 22:25
  • Thanks Enigma. I use .NET 4.0 but didn't use TPL before. I didn't think it would be this hard. If I update the UI in 1 step, it's easy but not desirable because the users want real time feedback as to what's going on :O – Joan Venge Mar 23 '11 at 22:27
  • @Joan - Making responsive UI is difficult. Using TPL you can asynchronously return all your results in one step. Using Rx you can asynchronously return each result, one at a time. Otherwise you'll need to allow the BW code to directly update the UI via calls invoked on the UI thread. All options are messy. Rx sounds like the best choice for you. It has operations to do background processing and foreground updating without too much work. – Enigmativity Mar 23 '11 at 22:36
  • Thanks Enigma, so returning results 1 by 1 like I want isn't even possible in TPL but only Rx? I need to finish this soon so I worry learning Rx is gonna take much more time once I get into it. – Joan Venge Mar 23 '11 at 22:41
  • @Joan - If you can post more of your code I can help you with the Rx code, if that helps. – Enigmativity Mar 23 '11 at 22:55
  • Thanks Enigma, it would help for sure. I can't post to SO, but I will see if I can extract some parts of it to make that part post-able. Do you use Rx alot in your own stuff? – Joan Venge Mar 23 '11 at 23:06
  • @Joan - Yes, I use Rx as a standard part of all my code. You could email me at my user name at gmail. – Enigmativity Mar 23 '11 at 23:22
  • Thanks man, appreciate it. I am onto something, so if I can do this, I will let you know, because then I won't need to do complex threading for the UI. – Joan Venge Mar 23 '11 at 23:23
  • Ok I managed to do this by using Action and do what I want with the passed EffectResult on each iteration, it works just like I wanted. Glad I didn't need to complicate it further. – Joan Venge Mar 24 '11 at 00:25
4

For new readers: the most elegant way to implement 'anonymous iterators' (i. e. nested in other methods) in C#5 is probably something like this cool trick with async/await (don't be confused by these keywords, the code below is computed absolutely synchronously - see details in the linked page):

        public IEnumerable<int> Numbers()
        {
            return EnumeratorMonad.Build<int>(async Yield =>
            {
                await Yield(11);
                await Yield(22);
                await Yield(33);
            });
        }

        [Microsoft.VisualStudio.TestTools.UnitTesting.TestMethod]
        public void TestEnum()
        {
            var v = Numbers();
            var e = v.GetEnumerator();

            int[] expected = { 11, 22, 33 };

            Numbers().Should().ContainInOrder(expected);

        }

C#7 (available now in Visual Studio 15 Preview) supports local functions, which allow yield return:

public IEnumerable<T> Filter<T>(IEnumerable<T> source, Func<T, bool> filter)
{
    if (source == null) throw new ArgumentNullException(nameof(source));
    if (filter == null) throw new ArgumentNullException(nameof(filter));

    return Iterator();

    IEnumerable<T> Iterator()
    {
        foreach (var element in source) 
        {
            if (filter(element)) { yield return element; }
        }
    }
}
Community
  • 1
  • 1
user1414213562
  • 5,583
  • 1
  • 14
  • 10
1

DoWork is of type DoWorkEventHandler which returns nothing (void), so it's not possible at all in your case.

manji
  • 47,442
  • 5
  • 96
  • 103
1

The worker should set the Result property of DoWorkEventArgs.

worker.DoWork += (s, e) => e.Result = GlobalGraph.Effects.Select(x => image.Apply(x));
Richard Schneider
  • 34,944
  • 9
  • 57
  • 73
  • Can I "tap into" this result like an IEnumerable? Because I want to update my UI as the effects are applied, regarding their results. – Joan Venge Mar 23 '11 at 21:27
  • You should setup a handler for RunWorkerCompleted. Then, foreach (var effect in (GlobalGraph.Effects) e.Result) ... – Richard Schneider Mar 23 '11 at 21:30
  • So you mean I should add the progress update and the UI update to RunWorkerCompleted? Because I have some separation between this class and the UI, with ViewModels, etc. – Joan Venge Mar 23 '11 at 21:34
  • Yes, the DoWork cannot touch the UI because it is not on the UI thread. The RunWorkerCompleted is invoked after the DoWork completes and is ON the UI thread; this is where you update the UI. Please check accept if this answer is ok. – Richard Schneider Mar 23 '11 at 21:42
  • Thanks Richard but RunWorkerCompleted is only raised once, right? I want to update the UI each time an effect is executed, to show its result and update the progress bar. – Joan Venge Mar 23 '11 at 21:45
  • Then why do this on a background thread. Just do the code on the UI thread with using a BackgroundWorker. – Richard Schneider Mar 23 '11 at 21:50
  • But the UI thread only calls this static method inside the static class that's responsible for applying effects. Is this not a good practice? Because otherwise the logic will be put into the UI code, that belongs to the EffectGraph type. – Joan Venge Mar 23 '11 at 21:58
1

Ok so I did something like this which does what I wanted (some variables omitted):

public static void Run ( Action<float, EffectResult> action )
{
    worker.DoWork += ( sender, e ) =>
    {
        foreach ( var effect in GlobalGraph.Effects )
        {
            var result = image.Apply (effect);

            action (100 * ( index / count ), result );
        }
    }
};

and then in the call site:

GlobalGraph.Run ( ( p, r ) =>
    {
        this.Progress = p;
        this.EffectResults.Add ( r );
    } );
Joan Venge
  • 315,713
  • 212
  • 479
  • 689
0

I wanted to supplement user1414213562's answer with an implementation of the ForEachMonad.

static class ForEachMonad
{
    public static IEnumerable<A> Lift<A>(A a) { yield return a; }

    // Unfortunately, this doesn't compile

    // public static Func<IEnumerable<A>, IEnumerable<B>> Lift<A, B>(Func<A, IEnumerable<B>> f) =>
    //     (IEnumerable<A> ea) => { foreach (var a in ea) { foreach (var b in f(a)) { yield return b; } } }

    // Fortunately, this does :)
    
    public static Func<IEnumerable<A>, IEnumerable<B>> Lift<A, B>(Func<A, IEnumerable<B>> f)
    {
        IEnumerable<B> lift(IEnumerable<A> ea)
        {
            foreach (var a in ea) { foreach (var b in f(a)) { yield return b; } }
        }
        return lift;
    }

    public static void Demo()
    {
        var f = (int x) => (IEnumerable<int>)new int[] { x + 1, x + 2, x + 3 };
        var g = (int x) => (IEnumerable<double>)new double[] { Math.Sqrt(x), x*x };
        var log = (double d) => { Console.WriteLine(d); return Lift(d); };

        var e1 = Lift(0);
        var e2 = Lift(f)(e1);
        var e3 = Lift(g)(e2);
        // we call ToArray in order to materialize the IEnumerable
        Lift(log)(e3).ToArray();
    }
}

Running ForEachMonad.Demo() produces the following output:

1
1
1,4142135623730951
4
1,7320508075688772
9
Nathan Chappell
  • 2,099
  • 18
  • 21