2

I'm building a list of Actions based on some other data. Each action should do a call to a method, and the list of actions should be performed in parallel. I have the following code that works just fine for parameterless methods:

private void Execute() {

    List<Action> actions = new List<Action>();

    for (int i = 0; i < 5; i++) {
        actions.Add(new Action(DoSomething));
    }

    Parallel.Invoke(actions.ToArray());
}

private void DoSomething() {
    Console.WriteLine("Did something");
}

But how can I do something similar where the methods have parameters? The following does not work:

private void Execute() {
    List<Action<int>> actions = new List<Action<int>>();

    for (int i = 0; i < 5; i++) {
        actions.Add(new Action(DoSomething(i))); // This fails because I can't input the value to the action like this
    }

    Parallel.Invoke(actions.ToArray()); // This also fails because Invoke() expects Action[], not Action<T>[]
}

private void DoSomething(int value) {
    Console.WriteLine("Did something #" + value);
}
i3arnon
  • 113,022
  • 33
  • 324
  • 344
TheHvidsten
  • 4,028
  • 3
  • 29
  • 62
  • 1
    I believe you need to use it in a lambda, because the syntax you're using evaluates the function. i.e. `new Action(i => DoSomething(i))`. EDIT: Actually that's not it, but I think you need to do something similar to bake your value into the function call. – j.i.h. Jun 17 '16 at 13:39
  • And you're still going to use a `List`, because you still want a parameterless delegate, yes? – j.i.h. Jun 17 '16 at 13:43
  • You don't want `Action` because you're not passing a parameter to the action _when it's invoked_. You're providing the parameter value as part of the delegate definition. If you just leave it as `Action` then both problems are solved. – D Stanley Jun 17 '16 at 13:44

5 Answers5

7

Just keep your actions variable as a List<Action> instead of a List<Action<int>> and both problems are solved:

private void Execute() {
    List<Action> actions = new List<Action>();

    for (int i = 0; i < 5; i++) {
        actions.Add(new Action(() => DoSomething(i)));         }

    Parallel.Invoke(actions.ToArray()); 
}

private void DoSomething(int value) {
    Console.WriteLine("Did something #" + value);
}

The reason you want Action is becuase you're not passing the parameter when the action is invoked - you're providing the parameter value as part of the delegate definition (note that the delegate parameter is changed to a lambda with no input parameters - () => DoSomething(i)), so it's an Action, not an Action<int>.

Keith Sirmons
  • 8,271
  • 15
  • 52
  • 75
D Stanley
  • 149,601
  • 11
  • 178
  • 240
  • Important part to note is `new Action(() => DoSomething(i))` rather than `new Action(DoSomething(i))`. You want the lambda in there, rather than just dumping the method call in there. – Mitch Jun 17 '16 at 13:52
3

There is a pitfall that I ran into when using an indexed forloop with the creation of threads.

When giving the Parameter i directly into the DoSomething method the Output is:

Did something #5

Did something #5

Did something #5

Did something #5

Did something #5

It is probably not what one wants when using a Loop and changing the counting variable. But if you save it temporarily into a new variable like:

class Program
{

    private static void Execute()
    {
        List<Action> actions = new List<Action>();

        for (int i = 0; i < 5; i++)
        {
            // save it temporarily and pass the temp_var variable
            int tmp_var = i;
            actions.Add(new Action(() => DoSomething(tmp_var)));
        }

        Parallel.Invoke(actions.ToArray());
    }

    private static void DoSomething(int value)
    {
        Console.WriteLine("Did something #" + value);
    }


    static void Main(string[] args)
    {
        Execute();
        Console.ReadKey();
    }
}

you actually get the counting variable in its full Beauty:

Did something #0

Did something #2

Did something #3

Did something #1

Did something #4

apparently in C# the variable survives the Loop (I don't know where) and when the thread is executed the Compiler will jump to the line

actions.Add(new Action(() => DoSomething(i)));

and take the value of i which it had when the Loop ended! If you would use i to index a List or array it would always lead to an OutOfBoundsException ! This drove me mad for a week until I figured it out

Community
  • 1
  • 1
Mong Zhu
  • 23,309
  • 10
  • 44
  • 76
0

As j.i.h. pointed out, you need to use it in a Lamba. Change your snippet to this:

private void Execute()
{
    List<Action> actions = new List<Action>();

    for (int i = 0; i < 5; i++)
    {
        actions.Add(() => DoSomething(i));
    }

    Parallel.Invoke(actions.ToArray());
}

private void DoSomething(int value)
{
    Console.WriteLine("Did something #" + value);
}

You can omit new Action(() => DoSomething(i)); and directly pass the Lamba to List.Add like this List.Add(() => DoSomething(i));

Sander Aernouts
  • 959
  • 4
  • 16
0

it seems like what you actually want is just:

private void Execute() {
    Parallel.For(0, 5, DoSomething);
}

private void DoSomething(int value) {
    Console.WriteLine("Did something #" + value);
}
Slai
  • 22,144
  • 5
  • 45
  • 53
0

Mong Jhu answer is perfect, I wanted to show how Action<int> can be used for same output.

private static void Execute1()
        {
            List<Action> actions = new List<Action>();
            for (int i = 0; i < 5; i++)
            {
                actions.Add(new Action(() => DoSomething(i)));
            }
            Parallel.Invoke(actions.ToArray());
        }
        private static void Execute()
        {
            List<Action<int>> actions = new List<Action<int>>();
            for (int i = 0; i < 5; i++)
            {
                actions.Add(new Action<int>((x) => DoSomething(i)));
            }
            for (int i = 0; i < actions.Count; i++)
            {
                Parallel.Invoke(() => { actions[0](0); });
            }
        }
Krtti
  • 62
  • 10