What exactly do you mean by “should not be repetitive”? If you mean that you don’t want to get any duplicates, then you should basically take a list of the numbers 1-20, shuffle them, and then grab one at a time from the head of the list. For an efficient way to shuffle a list, see this Stack Overflow answer.
If you just mean that your current attempt gives 5, 5, 5, 5, 5, 5, 1, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2 etc then chances are you’re creating a new instance of Random each time you pick a number: don’t do that. Each time you create an instance, it will use the current time as the “seed” for the random number generator (unless you specify one explicitly). That means if you create several instances in quick succession, each will get the same seed and therefore give the same sequence of numbers.
Instead, use a single instance of Random and reuse it. (Note that it’s not thread-safe though, which is a pain.) For instance:
private static readonly Random Rng = new Random();
public int NextNumber()
{
return Rng.Next(20) + 1;
}
That won’t be thread-safe, but let us know if that’s a problem. An alternative is sometimes to pass the Random into the method (which would normally be more complicated, of course):
public int NextNumber(Random rng)
{
return rng.Next(20) + 1;
}
then the caller can reuse the instance appropriately.
If you want a thread-safe way of generating random numbers, you might want to look at my StaticRandom class in MiscUtil.
(Note that using rng.Next(1, 21) would also work fine – I happen to prefer the version above as I think it reduces the guesswork about inclusive/exclusive boundaries, but it’s a matter of personal taste.)