Is this a good way to generate a string of random characters?

Question

I found this snippet of code that generates a string of random characters:
http://www.c-sharpcorner.com/UploadFile/mahesh/RandomNumber11232005010428AM/RandomNumber.aspx

But is there a more elegant/faster/more reliable way to do this? This seems to rely on the fact that the numbers 26-91 are valid characters given the current encoding.

/// <summary>
/// Generates a random string with the given length
/// </summary>
/// <param name="size">Size of the string</param>
/// <param name="lowerCase">If true, generate lowercase string</param>
/// <returns>Random string</returns>
private string RandomString(int size, bool lowerCase)
{
    StringBuilder builder = new StringBuilder();
    Random random = new Random();
    char ch;

    for(int i = 0; i < size; i++)
    {
        ch = Convert.ToChar(Convert.ToInt32(Math.Floor(26 * random.NextDouble() + 65)));
        builder.Append(ch);
    }

    if(lowerCase)
        return builder.ToString().ToLower();

    return builder.ToString();
}

Answer 1

I'd prefer to pass the Random instance into the method - then you can reuse the same instance multiple times, which is important if you need to generate lots of random strings in quick succession. However, I'd also modify it somewhat anyway:

public const string LowerCaseAlphabet = "abcdefghijklmnopqrstuvwyxz";
public const string UpperCaseAlphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";

public static string GenerateUpperCaseString(int size, Random rng)
{
    return GenerateString(size, rng, UpperCaseAlphabet);
}

public static string GenerateLowerCaseString(int size, Random rng)
{
    return GenerateString(size, rng, LowerCaseAlphabet);
}

public static string GenerateString(int size, Random rng, string alphabet)
{
    char[] chars = new char[size];
    for (int i=0; i < size; i++)
    {
        chars[i] = alphabet[rng.Next(alphabet.Length)];
    }
    return new string(chars);
}

• There's no need to use a StringBuilder when you know the final length
• Using Random.NextDouble() indicates a lack of knowledge of the Random class. (In particular Random.Next(int, int)
• Creating a new Random on each call is likely to result in duplicate strings
• Calling Convert.ToInt32 and Convert.ToChar seems ugly compared with just casting
• Lower-casing afterwards seems pointless compared with picking lower case letters to start with
• Providing an alphabet to pick from is a lot more flexible (with helper methods for common cases)

Answer 2

Did you read this article by Ayende? Just wondering :)

Answer 3

If I were to do this (and I have, but in Java, squirreled away somewhere), I would provide an array of allowable characters, and use a RNG simply to pick the index of the character. This would also let you disallow characters you don't want to generate (if you're creating a human-enterable license key, you don't want to generate characters that can be confused with each other; 0 and O, for instance, or 1 and l).

EDIT: yeah, like what Jon did...