Add Numbers by generating random tasks containing numbers

Question

public TestClass{

     public  Task<int> GetRandomNumber() {
                return Task.FromResult(new Random().Next(0, 1500));
            }
    }



public class Default
    {
        static void Main(string[] args)
        {
            var test = new TestClass();
            List<int> adddata = new List<int>();
            for (int i = 0; i < 3; i++)
            {
              var  result = Task.Run(() => test.GetRandomNumber());
              Console.WriteLine("The values that will be added are :{0}", result.Result);
              adddata.Add(result.Result);    
            }
            Console.WriteLine("The value is :{0}", adddata.Sum(v => v));
     }
}

The problem is random number returns same numbers,68 ,68, 122 when it should return distinct number what am I doing wrong I am trying to learn about how to use Task in C#.Thanks!

Btw. even if you don't create a new `Random` object each time (and solve the thread safety issue), "random" still does not mean "unique". So there still could be some same numbers (from time to time). If you do need *unique* random numbers, the usual approach is to have all possible numbers in a pool and then randomly take (and remove) numbers from that pool. In your case, have a `new List(Enumerable.Range(0, 1500))`, and go from there. — Corak, May 13 '15 at 14:57

Yuval Itzchakov · Accepted Answer · 2015-05-13T14:52:04.937

Because you're generating a new Random instance each time. Create the Random object once. As Random isn't thread safe, we'll need to use a random generator which is:

public static class ThreadSafeRandom 
{ 
    private static Random global = new Random(); 

    [ThreadStatic] 
    private static Random local;

    public static int Next() 
    { 
        Random inst = local; 
        if (inst == null) 
        { 
            int seed; 
            lock (global) 
            {
                seed = global.Next(); 
            }

            local = inst = new Random(seed); 
        } 

        return inst.Next(); 
    } 
}

And now consume it:

public TestClass
{
     public Task<int> GetRandomNumber() 
     {
         return Task.FromResult(ThreadSafeRandom.Next());
     }
}

Edit:

As a side note, you're executing your delegate on a threadpool thread and immediately synchronously blocking on it with Result. I'm assuming what you want to do is execute these in parallel:

static void Main(string[] args)
{
    var test = new TestClass();

    Task<int>[] addData = Enumerable.Range(0, 4)
                                    .Select(_ => Task.Run(() => test.GetRandomNumber()))
                                    .ToArray();

    Task.WaitAll(addData);
    foreach (var result in addData)
    {
        Console.WriteLine("The values that will be added are :{0}", result.Result);
    }

    Console.WriteLine("The value is :{0}", adddata.Select(x => x.Result).Sum());
}

Edit 2:

As per @ChrisL correctly pointing out Random isn't thread safe, I've modified the code to use the a thread-safe random generator provided by the PFX team.

The msdn article I linked states this is a common and incorrect solution - is that article wrong? — Chris L, May 13 '15 at 14:23
@ChrisL You're right. Yuval correctly points out that the OP is generating instances of `Random` in quick succession, but as your answer points out, generating one instance shared by all threads isn't the correct solution either. — dcastro, May 13 '15 at 14:37
@ChrisL You're right. I've modified the code with the thread-safe generator provided by the PFX team. Thanks for pointing that out, I've learned something as well :) — Yuval Itzchakov, May 13 '15 at 14:53
@downvoter - Would be nice to know what you find wrong about the answer. — Yuval Itzchakov, May 13 '15 at 15:01

score 0 · Answer 2 · answered May 13 '15 at 14:17

0

Look at this post here: Thread safe random number generation

In short: Random() is not thread safe, nor was it designed to be.

answered May 13 '15 at 14:17

Chris L

2,262
1
18
33

The instances of `Random` he's creating are local, they're not being shared. – dcastro May 13 '15 at 14:19
@dcastro You're right, I overlooked that. Thanks. – Yuval Itzchakov May 13 '15 at 14:21

Add Numbers by generating random tasks containing numbers

2 Answers2