0 
public static List<int> GetRandom()
{
    Random rnd = new Random();
    List<int> list = new List<int>();

    while (list.Count <= 26)
    {
        int randomNumber = rnd.Next(1, 26);

        if (!list.Contains(randomNumber))
        {
            list.Add(randomNumber);
        }
    }

    return list;
}

This is a code where I have tried to get a random list of integer(from 1 to 26) but this doesn't return me the desired result. Here I want a random int array without any repeat.

Improve this question
5
  • I think you want this one:stackoverflow.com/questions/30014901/… Commented Apr 23, 2019 at 18:43
  • 4
    Your rnd should be created once for the run of the application, not created on each call to the method. Commented Apr 23, 2019 at 18:44
  • You don't want them to be random only. You want them unique too. Commented Apr 23, 2019 at 19:20
  • @TheodorZoulias - Yes sir and i already got the answer Commented Apr 23, 2019 at 19:25
  • 1
    @Nishan you have the answer but your method is a bit inefficient. Because every iteration you will have a higer probability to generate an already existing number, so you probability will go from 1/n, 2/n...n-1/n... You will waste some time looping untill you actually generate unique number... Commented Apr 23, 2019 at 19:35

4 Answers 4

Reset to default
5

thats because you are trying to get numbers between 1-25, so your code will never leave loop. you should call random like this

int randomNumber = rnd.Next(1, 27);
Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

To expand, the first parameter to Next() is inclusive but the second parameter is exclusive. Meaning that the first parameter should be the lowest possible number in your range of desired numbers, and the second parameter should be the highest possible number + 1.
I think this will also result in an infinite loop since Nishan is doing while(list.Count <= 26)
Doesn't happen anything
@Nishan as a test, you can change your while loop condition to while(list.Count < 26), rather than <=. Alternatively, you could keep the while as-is and increase the second argument to the Next method, as outlined by Paulo, to 28. Otherwise you're still in an infinite loop.
@Sam You are right. It's working now. Thanks for your comment here
5

Actually, you want to randomize the range of integers. You could do this using System.Linq

Random rnd = new Random();
Enumerable.Range(1, 27).OrderBy(_ => rnd.Next())

.NET Fiddle

I even measure and compare two solutions using BenchmarkDotNet, even though I was pretty sure, just as confirmation. Two scenarios have been measured, the original and one with 1000 random elements. You could see performance degradation if you increase the number of the elements(which is logical as with the increase of the number of the elements you have an even higher probability to have the collision). 

BenchmarkDotNet=v0.11.5, OS=Windows 7 SP1 (6.1.7601.0)
Intel Core i7-6700 CPU 3.40GHz (Skylake), 1 CPU, 8 logical and 4 physical cores
Frequency=3328320 Hz, Resolution=300.4519 ns, Timer=TSC
[Host]     : .NET Framework 4.6.2 (CLR 4.0.30319.42000), 64bit RyuJIT-v4.7.2650.0
DefaultJob : .NET Framework 4.6.2 (CLR 4.0.30319.42000), 64bit RyuJIT-v4.7.2650.0

n=26

| Method |     Mean |     Error |    StdDev | Rank |
|------- |---------:|----------:|----------:|-----:|
|   Your | 4.463 us | 0.0882 us | 0.1936 us |    2 |
|   Mine | 2.597 us | 0.0235 us | 0.0220 us |    1 |

n=1000 

| Method |       Mean |       Error |      StdDev | Rank |
|------- |-----------:|------------:|------------:|-----:|
|   Your | 6,095.8 us | 119.4976 us | 122.7152 us |    2 |
|   Mine |   148.1 us |   0.6086 us |   0.5692 us |    1 |
Improve this answer

3 Comments

Can you tell me that which one is the best approach in terms of performance and why?
@Nishan The performances are degradated because during the Random.Next you might have collision so you waste one loop, with my proposal you don't have collision so the leading time is OrderBy complexity n*logn in average. See the update...
Got it. Thanks for your time
0

As an alternative you can use MathNet.Numerics library:

PM> Install-Package MathNet.Numerics

public static List<int> GetRandom()
{
    var arr = Combinatorics.GeneratePermutation(25);
    return new List<int>(arr);
}

You may need add 1, as it generates zero-based array.

Here is relevant documentation:

Generate a random permutation, without repetition, by generating the index numbers 0 to N-1 and shuffle them randomly. Implemented using Fisher-Yates Shuffling.

Improve this answer

Comments

0

Keeping the Random outside of the method as a static variable ensures that you'll always get a different list of numbers, even if you call the method many times in quick succession.

private static Random StaticRandom = new Random();

public static List<int> GetUniqueRandomNumbers_From_1_to_26()
{
    return Enumerable.Range(1, 26).OrderBy(_ => StaticRandom.Next()).ToList();
}

Usage example:

Console.WriteLine(String.Join(", ", GetUniqueRandomNumbers_From_1_to_26()));

Output:

26, 19, 22, 24, 16, 20, 5, 1, 8, 6, 10, 14, 13, 18, 15, 12, 25, 2, 4, 9, 21, 7, 23, 11, 3, 17
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.