21

Possible Duplicate:
c# random string generator

I need to generate a random string with a given length. This is my code so far. The problem is the random string is like "RRRRRR" or "SSSSS" The same letter every time. Just when i restart the app the letter change. I need something like "asrDvgDgREGd"

    public string GenerateChar()
    {
        Random random = new Random();

        return Convert.ToChar(Convert.ToInt32(Math.Floor(26 * random.NextDouble() + 65))).ToString();
    }

    public string GenerateChar(int count)
    {        
        string randomString = "";

        for (int i = 0; i < count; i++)
        {
            nahodneZnaky += GenerateChar();
        }

        return randomString;
    }
Community
  • 1
  • 1
user137348
  • 10,166
  • 18
  • 69
  • 89

11 Answers11

28

Try using the same random object for the whole string, rather than initializing one for each char.

The random object will generate a "pseudo-random" number, based on mathematical progressions starting from a "seed" number. You can actually get the same sequence of "random" numbers if you initialize the Random object to the same seed every time.

Now, when you initialize Random without specifying a seed, it'll take the computer's clock as seed. In this case, you're probably doing it fast enough that the clock hasn't changed from one initialization to another, and you get always get the same seed.

You're better off initializing the Random object in your function that generates the random string, and passing it as a parameter to the GenerateChar function, so that you're calling NextDouble() several times on the same instance of the Random object, instead of only once on different instances of it.

Daniel Magliola
  • 30,898
  • 61
  • 164
  • 243
  • 2
    +1 for explaining why his code gives that output. – Smalltown2k Oct 15 '09 at 14:51
  • Thank you. I still think he should go with James' suggestion for this particular case, but I believe understanding why this happened would be useful, since Random is a pretty common thing to use in other scenarios. – Daniel Magliola Oct 15 '09 at 16:18
  • A small improvement might be to also have a method called IsUpperCase which generates a random number between 0-1 – James Oct 16 '09 at 11:18
21

Don't create a new instance of Random on each iteration - that's going to seed each instance with the current time in milliseconds, which obviously isn't likely to change between iterations.

Create a single instance of Random, and pass it into the method. I'd also advise you not to concatenate strings like that in a loop, or even to create that many strings. Additionally, if you use Random.Next(int, int) to make your life a lot easier.

Try this:

public char GenerateChar(Random rng)
{
    // 'Z' + 1 because the range is exclusive
    return (char) (rng.Next('A', 'Z' + 1));
}

public string GenerateString(Random rng, int length)
{ 
    char[] letters = new char[length];
    for (int i = 0; i < length; i++)
    {
        letters[i] = GenerateChar(rng);
    }
    return new string(letters);
}

private static readonly Random SingleRandom = new Random();

public string GenerateStringNotThreadSafe(int length)
{ 
    return GenerateString(SingleRandom, length);
}

Now it's worth being aware that Random isn't thread-safe, so if you've got multiple threads you shouldn't just have a single instance of Random in a static variable without locking. There are various ways around this - either create a subclass of Random which is thread-safe, or a set of static methods which do the same thing, or use thread-local variables to have one instance per thread.

I have a StaticRandom class as part of MiscUtil but these days I'm leaning towards the thread-local version and passing it down the chain where appropriate. One day I'll add that as another option to MiscUtil...

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
20

You could save yourself the hassle and use Membership.GeneratePassword method. It is essentially doing what you require.

Usage:

int passwordLength = 5;
int alphaNumericalCharsAllowed = 2;
string random = Membership.GeneratePassword(passwordLength, alphaNumericalCharsAllowed);

For readability aswell you could wrap this in a helper method:

private string GenerateRandomString(int length, int alphaNumericalChars)
{
      return Membership.GeneratePassword(length, alphaNumericalChars);
}
James
  • 80,725
  • 18
  • 167
  • 237
  • the problem with this solution is although i set the second parameter to 0 there are still punctuation characters in the generated string. – user137348 Oct 16 '09 at 10:22
  • I looked into this, turns out the documentation isn't quite correct for this method, it does include punctuation (even though the documentation tells you otherwise!). In that case I would probably go with writing your own custom random string generator. – James Oct 16 '09 at 11:09
  • Alternatively you could use the GeneratePassword inside a for loop and check each generated password for punctuation until you find one without. Not ideal though, but would work. – James Oct 16 '09 at 11:15
  • Goes to show with the .NET API. I've been using Membership for 1+ years and didn't know about this method! +1 – Mark Struzinski Nov 19 '09 at 18:59
  • It’s a pity that there is no maxAlphaNumericalCharsAllowed parameter (which could be set to 0). But what I do is generate a longer password than I need using alphaNumericalCharsAllowed = 0 then strip out the stuff I don’t want. You need to loop this for the case where there’re too many non-alphanumerics but the overhead is minimal if you tune it properly. A good alternative tho. – SteveCinq Dec 20 '22 at 03:50
12

Nothing special, but short. 32 and 127 are min and max range of chars you want to be generated.

public static string GetRandomString(int length)
{
    var r = new Random();
    return new String(Enumerable.Range(0, length).Select(n => (Char)(r.Next(32, 127))).ToArray());
}
Kamarey
  • 10,832
  • 7
  • 57
  • 70
4

I know this might not be the best solution, but I kind of like the idea to be able to specify "allowed" characters:

    private const int _defaultNumberOfCharacters = 8;
    private const int _defaultExpireDays = 10;
    private static readonly string _allowedCharacters = "bcdfghjklmnpqrstvxz0123456789";

    public static string GenerateKey(int numberOfCharacters)
    {
        const int from = 0;
        int to = _allowedCharacters.Length;
        Random r = new Random();

        StringBuilder qs = new StringBuilder();
        for (int i = 0; i < numberOfCharacters; i++)
        {
            qs.Append(_allowedCharacters.Substring(r.Next(from, to), 1));
        }
        return qs.ToString();
    }
Tubbe
  • 1,068
  • 6
  • 22
  • I upvote this as I have found it essential to exclude "1" and "l" also "0" and "O" since users get these mixed up. – CodeGrue Nov 11 '11 at 19:25
1

http://msdn.microsoft.com/en-us/library/system.io.path.getrandomfilename.aspx

string randomName = Path.GetRandomFileName();
randomName = randomName.Replace(".", string.Empty);

// take substring...
juFo
  • 17,849
  • 10
  • 105
  • 142
1

Maybe you can find the Path.GetRandomFileName method useful, depending on the exact characters you need in the string.

Konamiman
  • 49,681
  • 17
  • 108
  • 138
1

Try static

public string GenerateChar()
{
    static Random random = new Random();

    return Convert.ToChar(Convert.ToInt32(Math.Floor(26 * random.NextDouble() + 65))).ToString();
}
Pavel Yakimenko
  • 3,234
  • 4
  • 28
  • 33
  • Careful! This is probably not thread-safe. If you're doing this in a web-app, you can run into weird threading issues (I learned this the hard way) – Daniel Magliola Oct 15 '09 at 16:20
  • Daniel Magliola, true. For this purpose you may lock generation: public string GenerateChar() { static Random random = new Random(); string rndString; lock (random) { rndString = Convert.ToChar(Convert.ToInt32(Math.Floor(26 * random.NextDouble() + 65))).ToString(); } return rndString; } – Pavel Yakimenko Oct 16 '09 at 06:35
0

Pass random from the calling method and initialise it once - you are reseeding the Random generator with the same seed each time....

penderi
  • 8,673
  • 5
  • 45
  • 62
0

You should initialize your random variable outside the method:

public Random random = new Random();

public string GenerateChar()
    {
       return Convert.ToChar(Convert.ToInt32(Math.Floor(26 * random.NextDouble() + 65))).ToString();
    }

...
Paulo Guedes
  • 7,189
  • 5
  • 40
  • 60
-1

Try this out.

public string GetRandomString()
{
    int randomNumber = new Random().Next();
    string RandomString = string.Empty;
    for(int i = 0;i < randomNumber.ToString().Length;i++)
    {
        RandomString += ((char)Convert.ToUInt32(randomNumber.ToString()[i])).ToString();
    }
    return RandomString;
}
moaz786
  • 40
  • 5