0

I have the following code:

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

class Task
{
        int _delay;
        private Task(int delay) { _delay = delay; }
        public void Execute() { Thread.Sleep(_delay); }
        public static IEnumerable GetAllTasks()
        {
            Random r = new Random(4711);
            for (int i = 0; i < 10; i++)
                yield return new Task(r.Next(100, 5000));
        }
 }

static class TaskExecuter
{
        public static void Execute()
        {
            foreach (Task task in Task.GetAllTasks())
            {
                 task.Execute();
            }
        }
   }

I need to change the loop in Execute method to paralle with multiple threads, I tried the following, but it isn't working since GetAllTasks returns IEnumerable and not a list

Parallel.ForEach(Task.GetAllTasks(), task =>
{
   //Execute();
});
Alex
  • 5,971
  • 11
  • 42
  • 80
  • Perhaps this will help: http://stackoverflow.com/questions/7617771/converting-from-ienumerable-to-list – DonBoitnott Jun 06 '13 at 11:28
  • @Giedrius thank you, this is right if it was an answer i would have given u 25 pts – Alex Jun 06 '13 at 11:40
  • You probably shouldn't call your class `Task`, to avoid confusion with `System.Threading.Tasks.Task`. – svick Jun 06 '13 at 12:04

3 Answers3

3

Parallel.ForEach works with IEnumerable<T>, so adjust your GetAllTasks to return IEnumerable<Task>.

Also .net has widely used Task class, I would avoid naming own class like that to avoid confusion.

Giedrius
  • 8,430
  • 6
  • 50
  • 91
2

Parallel.ForEach takes an IEnumerable<TSource>, so your code should be fine. However, you need to perform the Execute call on the task instance that is passed as parameter to your lambda statement.

Parallel.ForEach(Task.GetAllTasks(), task =>
{
    task.Execute();
});

This can also be expressed as a one-line lambda expression:

Parallel.ForEach(Task.GetAllTasks(), task => task.Execute());

There is also another subtle bug in your code that you should pay attention to. Per its internal implementation, Parallel.ForEach may enumerate the elements of your sequence in parallel. However, you are calling an instance method of the Random class in your enumerator, which is not thread-safe, possibly leading to race issues. The easiest way to work around this would be to pre-populate your sequence as a list:

Parallel.ForEach(Task.GetAllTasks().ToList(), task => task.Execute());

Douglas
  • 53,759
  • 13
  • 140
  • 188
  • `Parallel.ForEach()` may enumerate from multiple threads, *but not in parallel*. It uses locking to make sure only one thread enumerates at a time. So the usage of `Random` is fine. – svick Jun 06 '13 at 12:06
  • @svick: You're right; I was misled by the first reply in the linked post (which was written by an established contributor). Thanks for clarifying. – Douglas Jun 06 '13 at 12:13
0

This worked on my linqpad. I just renamed your Task class to Work and also returned an IEnumerable<T> from GetAllTasks:

class Work
{
        int _delay;
        private Work(int delay) { _delay = delay; }
        public void Execute() { Thread.Sleep(_delay); }
        public static IEnumerable<Work> GetAllTasks()
        {
            Random r = new Random(4711);
            for (int i = 0; i < 10; i++)
                yield return new Work(r.Next(100, 5000));
        }
 }

static class TaskExecuter
{
        public static void Execute()
        {
            foreach (Work task in Work.GetAllTasks())
            {
                 task.Execute();
            }
        }
   }
void Main()
{
    System.Threading.Tasks.Parallel.ForEach(Work.GetAllTasks(), new Action<Work>(task =>
{
   //Execute();
}));
}
svick
  • 236,525
  • 50
  • 385
  • 514
NeddySpaghetti
  • 13,187
  • 5
  • 32
  • 61