3

I'm generating many random numbers and I need a good function, since this doesn't help much:

public static class Randomizer
{
    static Random random = new Random((int)DateTime.Now.Ticks);

    public static int RandomInteger(int minimum, int maximum)
    {
        return random.Next(minimum, maximum + 1);
    }

    public static double RandomDouble()
    {
        return random.NextDouble();
    }
}

When I use this class, my numbers are very often the same. Do you have any simple idea how I can improve performance of the randomizer?

Thanks, Ivan

Ivan
  • 495
  • 3
  • 9
  • 20
  • 9
    Side note: Use `Environment.Ticks` instead of `DateTime.Now.Ticks`. Or, actually, don't provide a custom seed at all. – user541686 Jan 21 '11 at 15:16
  • I've tried that, but it didn't make much difference. – Ivan Jan 21 '11 at 15:17
  • 2
    @Ivan: Oh, it wasn't to make a difference. It's just that it's (a) not great practice to use the absolute time to do anything (because of how it depends on system time changes), and (b) unnecessary to pass the time, since it internally does that anyway. – user541686 Jan 21 '11 at 15:18
  • 5
    Are you calling this thing on multiple threads? – Eric Lippert Jan 21 '11 at 15:19
  • 2
    Are the numbers the same *more often than expected by chance*? – Eric Lippert Jan 21 '11 at 15:20
  • I'm using a single thread. It happens that I have a sequence of 10 to 20 same integers when the probability to choose an integer is less than 5%. – Ivan Jan 21 '11 at 15:21
  • 1
    @Ivan: Can you provide sample code of how you're calling it? – user541686 Jan 21 '11 at 15:21
  • 2
    The reason I ask about threading is because the documentation states that the class is not safe to be called from multiple threads. And in fact, one of its failure modes when called incorrectly can be to start spitting out the same number over and over again. – Eric Lippert Jan 21 '11 at 15:22
  • 2
    @Ivan, what minimum and maximum are you passing? If the difference is not wide enough, you will end up having duplicate. – Sam B Jan 21 '11 at 15:24
  • @Sam: Good point... I'll bet that's why he's getting duplicates. – user541686 Jan 21 '11 at 15:25
  • 5% is a set of 20 integers. You're going to see a lot of duplicates with such a small range. – Tergiver Jan 21 '11 at 15:25
  • I'm simulating diversity of objects. I gave probabilities for objects to have certain properties. Imagine I have to choose a color of a ball. I can choose between 5 colors. When I use this class, all balls are of the same color. In this case (and in most of the cases) i just choose a double between 0 and 1, but the numbers in sequences usualy don't differ for more than cca 0.05. – Ivan Jan 21 '11 at 15:28
  • 1
    @Sam, @Mehrdad: He should still see random distribution of values within the requested range no matter how small it is. There's something else going on here. – LukeH Jan 21 '11 at 15:33
  • @Tergiver: Yes, but of length ten? The chances of getting the same sequence of ten digits in a row twice if each digit has a 5% possibilitiy is one in 10000000000000. Something weird is going on here. – Eric Lippert Jan 21 '11 at 15:54
  • 2
    We need to see some code demonstrating the problem. I suspect the problem lies in the conversion from "probability of object to have a property" to code. – Tergiver Jan 21 '11 at 16:19
  • For posterity, I should mention that `System.Random` is a _terrible_ randomizer. See [this post](https://stackoverflow.com/a/54847919/2019892). – Kevin Mar 15 '19 at 10:19
  • if you in a single thread, try `new Random(int.Parse(DateTime.Now.ToStrring(fffffff)))` – Tyr Jan 04 '23 at 06:30

3 Answers3

12

See System.Security.Cryptography.RandomNumberGenerator

user541686
  • 205,094
  • 128
  • 528
  • 886
  • 1
    Click the link again. :) (Ever noticed the "Other Versions" dropdown?) – user541686 Jan 21 '11 at 15:24
  • Thanks a lot. Can you just tell me how to generate then a random number? I don't really manage to get what I want.. – Ivan Jan 21 '11 at 15:30
  • Use `RandomNumberGenerator.Create` to instantiate it, and then call `GetBytes` on the instance each time to fill an array of bytes. Then try using `BitConverter`'s methods to convert it to an integer. – user541686 Jan 21 '11 at 15:37
  • This will make it a lot slower, and more secure. But will it give better 'randomness'? – H H Jan 21 '11 at 17:32
  • It's "secure" precisely because of better randomness. – user541686 Jan 21 '11 at 17:50
  • 1
    Not when you use it wrong. I just meant the OP should have answered some of the comments instead of accepting this. – H H Jan 21 '11 at 18:46
  • 2
    @Henk: I agree -- I think the problem lied somewhere else. I answered his question, but I'm not sure if it's the solution that actually solved his problem. – user541686 Jan 21 '11 at 18:47
0

Take a look into MiscUtil from Jon Skeet.

There is a static random class, that works without any problems.

By the way, do you need random or unique numbers?

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Oliver
  • 43,366
  • 8
  • 94
  • 151
  • 1
    The only noticeable difference from the OP is locking for thread safety assuming OP is in a single threaded environment (big assumption) there will not be any difference. – David Waters Jan 21 '11 at 17:39
-3

The reason your numbers are the same is that you are creating a new Random every call to the class which means it will seed using the same number from the DateTime.Now.Ticks

You should make this a non static class. Instantiate it once and then call repeatedly the same function from the object

Richard
  • 21,728
  • 13
  • 62
  • 101
  • Since the random field in the class is static, it's only instantiated once. – Jon Benedicto Jan 21 '11 at 15:19
  • He don't create `new Random()` every call -- or he edited the question – abatishchev Jan 21 '11 at 15:20
  • No he's not - `static Random random` is only initialized once - where is he re-creating it? – BrokenGlass Jan 21 '11 at 15:20
  • True, or initiate "Random" in the methods like this: int randomInt = (new Random()).Next(min, max); – dampee Jan 21 '11 at 15:23
  • 2
    @dampee it **should** be only initialized once, it's a common mistake to do exactly what you just suggested, in that case you will get duplicate numbers. – BrokenGlass Jan 21 '11 at 15:26
  • @BrokenGlass. I posted my previous comment at the same time as yours. You are completely right! Looked it up on MSDN. (http://msdn.microsoft.com/en-us/library/system.random.aspx) Quote: Because the clock has finite resolution, using the parameterless constructor to create different Random objects in close succession creates random number generators that produce identical sequences of random numbers. – dampee Jan 21 '11 at 16:33