3

I'm attempting to create a randomly generated string of words, which works fine unless I call it multiple times in a row. This is on a WebForms page that and the list of words comes from a file.

I suspect I'm not understanding something with out C# or possibly ASP.NET works in this case, can someone explain why this is happening and how to approach a fix?

Here is the method

public string GeneratePhrase()
{
    // get dictionary file
    var data = File.ReadAllLines(HttpContext.Current.Server.MapPath("~/libs/words.txt"));

    Random rand = new Random();
    int r1 = rand.Next(data.Count());
    int r2 = rand.Next(data.Count());
    int r3 = rand.Next(data.Count());

    string p1 = data.ElementAt(r1).ToLower();
    string p2 = data.ElementAt(r2).ToLower();
    string p3 = data.ElementAt(r3).ToLower();

    string ret = string.Format("{0}{1}{2}", p1, p2, p3);
    return ret;
}

If I call this once during a PostBack, it's fine and always creates a random combination of words. However if I use this more than once during a PostBack, it simply repeats the same random string from each call.

string s1 = this.GeneratePhrase();
string s2 = this.GeneratePhrase();
string s3 = this.GeneratePhrase();
Response.Write(s1);
Response.Write(s2);
Response.Write(s3);

output

tirefriendhotdog
tirefriendhotdog
tirefriendhotdog

Any reason on why this may happen?

Kirk
  • 16,182
  • 20
  • 80
  • 112
  • 2
    Call `Random rand = new Random();` only once. Try moving that line outside of that scope. – LarsTech Mar 24 '16 at 18:10
  • 3
    I suspect this is all taking place *very* quickly and the three different `Random` objects have the same exact seed values. – David Mar 24 '16 at 18:11
  • I like that your problem is my solution, I was having the exact opposite problem – GibsonFX Jul 07 '19 at 01:41

2 Answers2

6

Random rand = new Random(); uses the current time as a seed value, so calling it multiple times in quick succession will give you the same random sequences. You can:

  • create a single Random object that is used for all requests
  • seed it with a different pseudo-random number like Guid.NewGuid().GetHashCode()
  • Use a different random number generator that is not time-dependent.
D Stanley
  • 149,601
  • 11
  • 178
  • 240
4

MSDN has this to say about the parameterless constructor for Random:

The Random() constructor uses the system clock to provide a seed value. This is the most common way of instantiating the random number generator.

When you create several new instances of Random in short succession, they may end up with the same seed, and so produce the same sequence.

To get proper (pseudo)randomness, it is best to have only a single instance of Random per thread in you application.

Jason Watkins
  • 3,766
  • 1
  • 25
  • 39