Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have some code that run a series of functions in order and then outputs the results of each. When I execute this code, I am reusing a ton of it, where only the function itself in the code is changing. MethodThatChanges in the below example, is a method that returns void.

Task[] task1 = new Task[10];
for (int i = 0; i < 10; i++)
{
    task1[i] = Task.Factory.StartNew(() =>
    {
        MethodThatChanges1(i);
    });
}
Task.WaitAll(task1);

Task[] task2 = new Task[10];
for (int i = 0; i < 10; i++)
{
    task2[i] = Task.Factory.StartNew(() =>
    {
        MethodThatChanges2(i);
    });
}
Task.WaitAll(task2);

etc........

I would like to write a function instead that contains all of this code and passes in as a parameter, the method that each section calls. Something like this, obviously this doesn't work as is...

void ExecuteTask(void method)
{
    Task[] task = new Task[10];
    for (int i = 0; i < 10; i++)
    {
        task[i] = Task.Factory.StartNew(() =>
        {
            method(i);
        });
    }
    Task.WaitAll(task);
}

Then I could replace all of the copied code with:

ExecuteTask(method1);
ExecuteTask(method2);
etc...

I've looked into Action and Func delegates, but I do not understand exactly how that work. Any help would be appreciated.

share|improve this question

closed as off-topic by RubberDuck, Vogel612, Marc-Andre, Simon André Forsberg, Edward Jul 25 at 13:11

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions must involve real code that you own or maintain. Questions seeking an explanation of someone else's code are off-topic. Pseudocode, hypothetical code, or stub code should be replaced by a concrete example." – RubberDuck, Vogel612, Marc-Andre, Simon André Forsberg, Edward
If this question can be reworded to fit the rules in the help center, please edit the question.

    
Welcome to Code Review! Unfortunately, it's a bit hard to review code like this because the code you are showing is example code rather than code that you're actually using in a real project, which means that a lot of context has been stripped away. Unlike Stack Overflow, we prefer to look at real code instead of example code. –  Simon André Forsberg Jul 25 at 12:43

2 Answers 2

up vote 2 down vote accepted

You were on the right track with Action delegates. There is another problem however: the loop variable i is captured inside the lambda expression. Since it is the same variable in each Task, when you do i++ in the for-loop, other Tasks will use the new value as well.

(This is a very weird problem, more on this on Eric Lippert's blog: Closing over the loop variable considered harmful) It can be solved by simply creating a copy of i.

void ExecuteTask(Action<int> method) {
    Task[] task = new Task[10];
    for (int i = 0; i < 10; i++)
    {
        int copy = i;
        task[i] = Task.Factory.StartNew(() => method(copy));
    }
    Task.WaitAll(task);   
}

ExecuteTask(method1);
ExecuteTask(method2);
share|improve this answer

An Action variable would be what you're looking for as it appears as though your method doesn't return anything. (If it returns something, use a Func.) So create another method that will do the following:

void ActionMethod(Task[] tasks, Action<int> method) {
    for (int i = 0; i < tasks.Length; i++) {
        tasks[i] = Task.Factory.StartNew(() => method(i));
    }
}

Then you can call it from your other methods like so:

Task[] tasks1 = new Task[10];
Task[] tasks2 = new Task[10];

ActionMethod(tasks1, MethodThatChanges1);
ActionMethod(tasks2, MethodThatChanges2);
share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.