Let's look at some of the code:
Random random = new Random((int)DateTime.Now.Ticks);
You don't need to create a seed for the Random constructor from the clock, the parameterless constructor does that:
Random random = new Random();
List<string> characters = new List<string>() { };
You don't need the initialiser brackets when you don't put any items in the list at that point:
List<string> characters = new List<string>();
result += characters[random.Next(0, characters.Count)];
Using +=
to concatenate strings is bad practice. Strings are not appended at the end, as strings are immutable. Code like x += y;
actually end up as x = String.Concat(x, y)
. You should rather use a StringBuilder
to build the string.
Thread.Sleep(1);
Why on earth are you sleeping in the middle of the loop?
Instead of creating a list of strings, just use a string literal to pick characters from:
public static string GenerateCoupon(int length) {
Random random = new Random();
string characters = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
StringBuilder result = new StringBuilder(length);
for (int i = 0; i < length; i++) {
result.Append(characters[random.Next(characters.Length)]);
}
return result.ToString();
}
Consider if all those characters should be included, or wheter you should omit similar looking characters like o
, O
and 0
. Having the characters in a string literal makes it easy to do that.
Edit:
If you are going to call the method more than once, you should send in the random generator as a parameter:
public static string GenerateCoupon(int length, Random random) {
string characters = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
StringBuilder result = new StringBuilder(length);
for (int i = 0; i < length; i++) {
result.Append(characters[random.Next(characters.Length)]);
}
return result.ToString();
}
Usage:
Random rnd = new Random();
string[] coupon = new string[10];
for (int i = 0; i < coupon.Length; i++) {
coupon[i] = GenerateCoupon(10, rnd);
}
Console.WriteLine(String.Join(Environment.NewLine,coupon));
Example output:
LHUSer9dPZ
btK0S01yLb
hruw4IXINJ
hwMdRDRujt
cr4TDezvcZ
b8tVETNXNL
JrG6sfXgZF
Y7FRypnRiQ
JbfnhY3qOx
quWNakbybY
I
(upper-case I),l
(lower-case L) and1
(the number 1) appear? \$\endgroup\$